Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
At the moment, tagadelic will run this query a NUMBER of times:
SELECT v.vid, v.*, n.type FROM vocabulary v LEFT JOIN vocabulary_node_types n ON v.vid = n.vid WHERE n.type = 'WHATEVER' ORDER BY v.weight, v.name
This is because the function tagadelic_node_get_terms($node) calls taxonomy_get_vocabularies, which generates a query each single time. In my home page, this means several extra queries in pages with several nodes.
This patch makes sure that tagadelic_node_get_terms($node) caches the result of taxonomy_get_vocabularies, therefore avoiding several useless DB calls.
Bye!
Merc.
Comment | File | Size | Author |
---|---|---|---|
#13 | 193057_tagadelic_less_queries_d6.patch | 719 bytes | mrfelton |
#9 | tagadelic_less_queries_193057.patch | 1.06 KB | Jo Wouters |
perf.patch | 691 bytes | mercmobily |
Comments
Comment #1
mercmobily CreditAttribution: mercmobily commentedHi,
Apologies... I've just noticed this:
http://drupal.org/node/185017
Now... I have this patch ready, and it works. However, that other issue is marked as "postponed". Is there any particular reason not to accept this patch?
Bye,
Merc.
Comment #2
mercmobily CreditAttribution: mercmobily commentedHi,
Is anybody maintaining this module?
Is this patch likely to make it?
Bye,
Merc.
Comment #3
Bèr Kessels CreditAttribution: Bèr Kessels commentedwhy are you not reusing the variable $vocs?
Comment #4
mercmobily CreditAttribution: mercmobily commentedHi,
I do not understand your question.
The patch brings minimal disruption to the module's logic. Basically, rather than having:
$vocs = taxonomy_get_vocabularies($node->type);
You check that you didn't make that call already:
$vocs = taxonomy_get_vocabularies($node->type);
+ if(!isset($vocabulary[$node->type])){
+ $vocabulary[$node->type]=taxonomy_get_vocabularies($node->type);
+ }
+ $vocs = $vocabulary[$node->type];
The variable $voc is being "used" just the same as before, anda new variable - the cache, $vocabulary - is being added.
If you ask me, this patch makes sense and it's being used on a production server right now.
Bye,
Merc.
Comment #5
mercmobily CreditAttribution: mercmobily commented...?
Comment #6
Bèr Kessels CreditAttribution: Bèr Kessels commentedThe code still seems odd to me. I really don't see the advantage of adding extra variables that contain exactly the original content, for the sake of "less disruption". Lets focus on good code. Not on explaining history --codewise--
Comment #7
mercmobily CreditAttribution: mercmobily commentedHi
I proposed a solution to a tangible problem of the code.
You didn't like the solution I proposed, which is fair enough.
I can't think of a better way of doing it. In fact, I think the code I wrote is exactly how it should be done, disruprion or not-disruption.
Right now the module repeats queries, which is a problem. If you don't like my solution (I don't see why not, but you must have some reasons), the only solution is for you to think of a better way to fix the module's problem, which is tangible and "
there".
Bye,
Merc.
Comment #8
mercmobily CreditAttribution: mercmobily commentedHi,
Any news for a better solution to this bug?
Merc.
Comment #9
Jo Wouters CreditAttribution: Jo Wouters commentedThis patch reuses $vocs
Comment #10
Bèr Kessels CreditAttribution: Bèr Kessels commentedDid anyone test this on the RC1?
If not, I will release 5.x-1.0 first and then include this patch in the new HEAD. Making it a feature for the 5.x-1.1 (or, in case it comes earlier, the 6.x-1.0) release.
Comment #11
neochief CreditAttribution: neochief commentedPatched my module with this patch and all seems to be fine. Static caching rocks :)
Comment #12
wmostrey CreditAttribution: wmostrey commentedI can confirm that this patch applies cleanly on 5.x-1.0. For us this made the difference between patching the module and keeping it enabled on a high traffic site, or disabling it.
Comment #13
mrfelton CreditAttribution: mrfelton commented+1. This vastly reduces the number of duplicate queries from about 150 to 8 in my case. Patch at #9 ported to Drupal 6.
Comment #14
Bèr Kessels CreditAttribution: Bèr Kessels commentedcommitted to 6, HEAD (to be 7) and closing. Thanks