This is a great module, but he use 100+ queries for my frontpage! It is too much. More that all my blocks...

CommentFileSizeAuthor
#4 readmore_on.txt75.17 KBsuperfedya
#4 readmore_off.txt57.23 KBsuperfedya

Comments

todd nienkerk’s picture

Status: Active » Postponed (maintainer needs more info)

Can you give me more specific information? The module doesn't make any direct queries, and a limited number of calls to variable_get(). Also, the node object is grabbed in template_preprocess_node() without any loading or viewing.

Are you positive the queries are the result of this module?

Also, I think comparing node-related queries to block-related ones may be a poor benchmark. You almost certainly have more node calls on your front page than blocks.

superfedya’s picture

I have 15 items (teasers) in my frontpage.
Read more disable: Executed 281 queries in 147.3 milliseconds
Read more enable: Executed 391 queries in 214.59 milliseconds

tested many times.

If You want, I can post the list of queries.

todd nienkerk’s picture

Yes, please post the queries.

superfedya’s picture

StatusFileSize
new57.23 KB
new75.17 KB

Here

todd nienkerk’s picture

Version: 6.x-5.0-rc3 » 6.x-5.0-rc5

After carefully comparing your list of queries, I've found that the following are added per node as a result of enabling Read More link:

node_token_values
SELECT name, mail FROM users WHERE uid = 45
0.16
1
node_token_values
SELECT mlid FROM menu_links WHERE link_path = 'node/186'
0.31
1
pathauto_token_values
SELECT t.vid FROM term_node r INNER JOIN term_data t ON r.tid = t.tid INNER JOIN vocabulary v ON t.vid = v.vid WHERE r.vid = 186 ORDER BY v.weight, t.weight, t.name LIMIT 0, 1
0.25
1
pathauto_token_values
SELECT t.tid, t.name FROM term_data t INNER JOIN term_node r ON r.tid = t.tid WHERE t.vid = 1 AND r.nid = 186 ORDER BY weight LIMIT 0, 1
0.13
1
taxonomy_get_term
SELECT * FROM term_data WHERE tid = 10
1.66
7
taxonomy_get_parents
SELECT t.tid, t.* FROM term_data t INNER JOIN term_hierarchy h ON h.parent = t.tid WHERE h.tid = 10 ORDER BY weight, name
1.38
8
taxonomy_get_parents
SELECT t.tid, t.* FROM term_data t INNER JOIN term_hierarchy h ON h.parent = t.tid WHERE h.tid = 5 ORDER BY weight, name
0.25
1
drupal_lookup_path
SELECT dst FROM url_alias WHERE src = 'taxonomy/term/10' AND language IN('ru', '') ORDER BY language DESC
0.25
1

This is consistent with your finding that roughly 100 queries are being added: 8 queries per node * 15 nodes = 120.

All of these are are a result of adding token support. For each node that's being loaded, the token system is looking up information related to taxonomy, pathauto, and other commonly user tokens.

I don't know how to fix this offhand, and I'm not convinced this is bad. Someone with more performance experience than I would need to look at the queries and determine if they are significantly impacting performance.

rhouse’s picture

Who needs token support for a trivial module like this? All we want to do is shift the Read More message to a different spot. The module is clearly getting too big for its boots. Everything for me was working fine with the old version, until suddenly I started getting big red coloured security warnings about it - not supported or something. (Apparently this was harmless, but I cannot have my admin pages permanently showing red warning because if I ignore them, a genuine security problem will appear on another module and I won't see it.) So instead of simply fixing whatever minor issue was going wrong, the whole thing has been rewritten to provide heaps of new "features" that are useless to 99% of us and aren't working properly anyway. It isn't smart.

Sorry for the agro, but I've been fighting spammers and this is the last thing I need. I do appreciate your hard work and I thank you for it.

todd nienkerk’s picture

RTH: Your comment is off-topic, but I'd like to address some points.

(1) The "heaps" of new features presumably include customizable insertion points (elements) and customization of the link element attributes (title, nofollow, etc.) that are useful for people who are concerned with accessibility and SEO. (A customizable "title" attribute, for example, is important for screen readers and search engines.)

(2) Tokens are absolutely necessary for fully customizable title attributes and were requested more than once. Tokens allow a site administrator the ability to dynamically add node titles and other information to both the link text and title attributes. (For example, Read more can read Continue reading "My great article...".)

(3) Apart from three bugs — one involving the insertion of non-breaking spaces, one involving prepended CCK output, and the other a conflict with some yet-unknown module — these new features do work. Two of those were promptly fixed thanks to constructive feedback from users (and overtime on my part). Saying all of the new features "aren't working properly" is inaccurate and deliberately confrontational.

(4) Please provide data that demonstrate the new features are "useless to 99%" of users.

If you indeed find the improvements to the module to be so terrible, please revert to the 6.x-3.0 release and stop using the issue queue as a means of venting your frustration. I have nothing to do with your spam problems, and I have a hard time taking your thanks and appreciation seriously when it's preceded by an angry, exaggerated diatribe about how my efforts at accommodating reasonable user requests "isn't smart."

rhouse’s picture

I was perfectly happy with the old release but it now makes all my admin pages show a red warning about modules. Other out of date modules just show in yellow. Since that red is (or should be) only used for security issues, it means I have to manually check every day in case a real security issue has popped up somewhere else. It is not possible to revert, so my only option is to run a non-standard version.

As for Continue reading "My great article..." - do you think one in a 100 websites wants that kind of distraction? I would argue that as a matter of fact it is worse than a standard message. Based on my own web surfing, I think useless for 99% is heaps conservative in that case. Vastly increasing the SQL count for everyone is not the best balance between features and cost. Can a simple preference variable allow opting out of calling the token rewriting code, thereby skipping the DB calls?

And I do appreciate work done for free out of generosity, even if it does cause some hair-pulling moments! ;-)

superfedya’s picture

>I was perfectly happy with the old release

Me too.

todd nienkerk’s picture

Since that red is (or should be) only used for security issues [...]

It's also used to alert the site maintainer that a module is no longer supported, which was the case here. The 6.x-3.0 release is no longer marked unsupported.

It is not possible to revert [...]

Yes, it is. Simply download the 6.x-3.0 release and overwrite the files. If you want to be completely thorough, disable the 6.x-5.x version and uninstall it (via the "Uninstall" tab) to remove all stored variables prior to overwriting the files and re-enabling the module.

As for Continue reading "My great article..." - do you think one in a 100 websites wants that kind of distraction?

Again, there are perfectly valid use cases for this. Inserting the node title — especially into the title="" attribute — is very useful for accessibility and SEO purposes. A verbose, contextual attribute helps sighted users navigate the page. Instead of hearing a dozen links that say simply "Read more," that user can hear exactly what article or post that link refers to.

todd nienkerk’s picture

Title: SQL Queries usage - too much? » Addition of Token support adds eight DB queries to each teaser
Assigned: Unassigned » todd nienkerk
Category: task » feature
Status: Postponed (maintainer needs more info) » Fixed

Release 6.x-5.0-RC6 has an option to disable Token support and bypass all token-adding logic.

After consulting with a performance expert, we determined this is fundamentally a problem with the Token module, which runs various queries regardless of whether token replacement strings actually exist. The option added to this module will be remove when the Token module can be modified to parse strings prior to running queries.

mcurry’s picture

@Todd:

Thanks for adding the option to disable the token support (I do like that feature, but I'll have to wait for performance issues to be sorted out...)

And, thanks again for your continued efforts in maintaining and enhancing this module. Please let me know how I can help.

todd nienkerk’s picture

grateful_drupal_user: To clarify the performance question, the expert I talked to didn't think the added queries were a big deal. In his opinion, an eight additional queries per node is not an unreasonable amount -- especially considering they're coming from Token.

He did, however, think that Token should check strings for the presence of tokens prior to running its queries. The problem, then, lies in Token. There's nothing I can do about it here except provide an option to disable Token support and prevent unnecessary queries from being run.

eaton’s picture

Token shouldn't be triggering any additional queries unless you've installed other modules that load piles of extra tokens for each node. If it's causing performance problems, though, unfortunately the best solution is to either remove the token support or make it configurable: i.e., 'use tokens for this text.'

The original intent of the token support was to allow accessibility-minded administrators to control the precise text of the 'title' attribute of the read more link, but that may be an edge case.

The Token mechanism that is built into Drupal 7 provides a much cleaner system: it first scans for the existence of tokens, then generates the replacement text.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.