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.
Location module creates queries which perform full table scan of locales_source table:
SELECT s.lid, t.translation FROM locales_source s INNER JOIN locales_target t ON s.lid = t.lid WHERE s.source = 'Users can use their own name or handle and can fine tune some personal configuration settings through their individual my account page. Registered users need to authenticate by supplying either a local username and password, or a remote username and password such as DelphiForums ID, or one from a Drupal powered website. A visitor accessing your website is assigned an unique ID, the so-called session ID, which is stored in a cookie. For security\'s sake, the cookie does not contain personal information but acts as a key to retrieve the information stored on your server. ' AND t.locale = 'pl'
Solution:
Add index on 'source' field for 'locales_source' table
(for MySQL)
ALTER TABLE `schema_name`.`locales_source` DROP INDEX `source`,
ADD INDEX `SOURCE`(`source`(30));
Comment | File | Size | Author |
---|---|---|---|
#43 | locale_index.patch.txt | 2.19 KB | Heine |
#19 | locales_speed_improvement.patch | 2.67 KB | killes@www.drop.org |
#11 | 42463_ab_log | 2.95 KB | Bèr Kessels |
#9 | locale_speed_improvement.diff | 2.3 KB | Stefan Nagtegaal |
#3 | locales_speed_improvement.diff | 2.26 KB | Stefan Nagtegaal |
Comments
Comment #1
janekb CreditAttribution: janekb commentedProblems are only when location module is enabled and there are some untranslated strings. In such case no caching is performed and Drupal atempts to look up translation in this table.
Patch hasn't been tested with PostgresSQL (I'm not shure if index can be created on 'text' type fields).
Comment #2
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedThanks, can you provide a patch that includes an update for updates.inc?
Comment #3
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedHere it is, a new formed patch includes:
- updates database.mysql and database.pgsql;
- updates update.inc!
This does _really_ speed up your site when using the locale.module..
Please try, and see if we can get this little speed improvement into 4.7
Comment #4
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedAs an example:
I took at the following query:
"SELECT s.lid, t.translation FROM locales_source s INNER JOIN locales_target t ON s.lid = t.lid WHERE s.source = '\n <p>Welcome to your new <a href=\"%drupal\">Drupal</a>-powered website. This message will guide you through your first steps with Drupal, and will disappear once you have posted your first piece of content.</p>\n <p>The first thing you will need to do is <a href=\"%register\">create the first account</a>. This account will have full administration rights and will allow you to configure your website. Once logged in, you can visit the <a href=\"%admin\">administration section</a> and <a href=\"%config\">set up your site\'s configuration</a>.</p>\n <p>Drupal comes with various modules, each of which contains a specific piece of functionality. You should visit the <a href=\"%modules\">module list</a> and enable those modules which suit your website\'s needs.</p>\n <p><a href=\"%themes\">Themes</a> handle the presentation of your website. You can use one of the existing themes, modify them or create your own from scratch.</p>\n <p>We suggest you look around the administration section and explore the various options Drupal offers you. For more information, you can refer to the <a href=\"%handbook\">Drupal handbooks online</a>.</p>' AND t.locale = 'nl'"
It takes 20.22 ms to execute before this patch was in. After that, it only took 1.02 ms..
That would mean that the query has been executed 20 times faster as before, which means a tremendous speed up for sites which do use the locale.module..
Steef
Comment #5
Cvbge CreditAttribution: Cvbge commentedPostgresql part looks good, but you should use
switch
statement instead ofif
and also includemysqli
case (same as mysql), thus setting to 'needs work'Comment #6
Zen CreditAttribution: Zen commentedWould this also be a good place to check/find out if there is any reason why we use a BLOB here (MySQL) instead of TEXT ?
-K
Comment #7
Zen CreditAttribution: Zen commentedCaveats:
You can add an index on a column that can have NULL values only if you are using MySQL 3.23.2 or newer and are using the MyISAM, InnoDB, or BDB storage engine. This is also true for MEMORY tables as of MySQL 4.0.2. You can only add an index on a BLOB or TEXT column if you are using MySQL 3.23.2 or newer and are using the MyISAM or BDB storage engine, or MySQL 4.0.14 or newer and the InnoDB storage engine.
The field length also bears investigating along with the NULL/duplicates issue.
Cheers
-K
Comment #8
Zen CreditAttribution: Zen commentedComment #9
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedWould this also be a good place to check/find out if there is any reason why we use a BLOB here (MySQL) instead of TEXT ?
@Zen: IMO that should be a new issue. I don't want to cover more than absolutely nessecary in this issue because I would like to see this committed against HEAD before release of 4.7.
@Zen: What do you ment with your comment about the caveats? I don't know if I understand what you mean.. Does it give you any errors?
@Cvbge: Formatting of updates.inc has been improved as you pointed out.
Both, thanx for the review, new patch is attached.
Comment #10
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedSetting status back to "patch (code needs review)"..
Comment #11
Bèr Kessels CreditAttribution: Bèr Kessels commentedI did an apache benchmark. And attached the outcome. AFAIK the patch had no real effect, the unpatched version looks faster even.
Disclaimer, i am clueless when it comes to benchmarks ;)
Comment #12
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedCan you do the test again with ab -n 200 -c1? Your test did indeed no tell very much. You did have locale enabled, did you? I think the patch will only affect pages where there are long strings (strlen > 75) which will be retrieved directly from the db.
Comment #13
Zen CreditAttribution: Zen commentedhttp://drupal.org/node/50669 <-- This patch makes MySQL support engine agnostic, basically stating that Drupal will work on all (common) engines on MySQL v3.23.58+. As stated above, innoDB does not support TEXT/BLOB indexing in versions before 4.0.14, thus making this patch rather dicey.
Just a thought.
-K
Comment #14
Cvbge CreditAttribution: Cvbge commentedAnd: have you had cache enabled? (should be disabled). How many translations (different languages) you had imported? (I suppose 1 non-english translation is most common, but testing with 2-3 could be good too).
Also, I wonder if index on the first 30 characters is optimal.
Comment #15
Zen CreditAttribution: Zen commentedThe rest of my comment appears to have been eaten up.
The issue in my last statement makes MySQL support in Drupal engine agnostic. So we are essentially supporting all mainstream MySQL engines (and innoDB is one of them) on versions of MySQL in v.3.23.58 onwards. As stated above, innoDB does not support TEXT/BLOB field indexing on MySQL versions older than 4.0.14, thereby making this patch a little on the dicey side.
Cheers
-K
Comment #16
Bèr Kessels CreditAttribution: Bèr Kessels commentedI will try to rerun that benchmark later.
1) no cache enabled
2) I ran it on ?q=tracker. I am not aware of any page with large strings that are available to anonymous. Ill create a php page for that.
3) I did the c4 on purpose; in my system the slow queries, those wayting while the lock is on affect speed most.
4) modules: all standard modules + locale + statsictics (the imo one-but biggest slow-downer)
5) 3 translations. approx 600 strings 'imported'.
6) only four strnigs were translated. That could be the cause?
Bèr
Comment #17
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedif the locale_target table is empty an index cannot have an impact.
Comment #18
chx CreditAttribution: chx commentedtavon tested changing blob to text in both source and target tables and it's ok.
Comment #19
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI've included the change the blob to text change
Comment #20
Steven CreditAttribution: Steven commentedAFAIK we use blob because mysql treats text in a case insensitive fashion.
Comment #21
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedDon't think so:
http://dev.mysql.com/doc/refman/4.1/en/blob.html
BLOB columns have no character set, and sorting and comparison are based on the numeric values of the bytes in column values. TEXT columns have a character set, and values are sorted and compared based on the collation of the character set assigned to the column as of MySQL 4.1. Before 4.1, TEXT sorting and comparison are based on the collation of the server character set.
Comment #22
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedSeems Steven was right and I wasn't.
we have two choices:
1) not apply the patch
2) apply it (minus the blob2text change), and change the system requiremts to state that you can't use old innodb and locales.
Comment #23
killes@www.drop.org CreditAttribution: killes@www.drop.org commented3) make the locales_source table explicitly MyISAM again.
Comment #24
yched CreditAttribution: yched commentedI run a french localized site. A lot of admin pages are not translated, and the views/edit page for instance (quite heavy, all untranslated strings) was taking ages to build.
I tested the patch (well I just created the index - no blob2text thing)
This is in no way a regular benchmark - i just rely on query times displayed by devel.module
Before :
40 to 50 ms (!!) for every locale query
After :
all these queries drop to 2 or 3 ms...
local server (Win), MySQL5
one language and 3000 records in locale_target
I guess my personnal opinion is settled : commit it or not, I keep it ! :-)
Naive question : why don't we store in cache the fact that a string has no translation ?
We have to go through all those unsuccessful queries on each page load ?
views/edit page triggers something like 200+ of these queries, that's half a second...
Comment #25
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedSo killes, what method do you prefer and would let you commit this to HEAD before the release of drupal 4.7?
I'll write the patch which is needed, if time permits...
Steef
Comment #26
Zen CreditAttribution: Zen commentedThis has been postponed to 4.8 - I've created a documentation issue so that this goes into the handbooks for now.. http://drupal.org/node/56943
-K
Comment #27
Dries CreditAttribution: Dries commentedLet's get this into the next version!
Comment #28
Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commentedIIRC the problem was that adding an index on blob colums wasn't supported for all database backends we currently support. IIRC, InnoDB on mysql 3 was not supported.
Comment #29
jadwigo CreditAttribution: jadwigo commented+1 for getting some work on this ... the speed improvements are really cool > from ~2500ms to ~550ms page generation time on /node for my homepage for example, more than 4 times faster, so less load on the server, so my hosting provider is happier too
IMO it's really important too fix this at least for incomplete translations, because many translations are incomplete as soon as you use contrib modules in a site. For example translating of admin pages is not a priority for most modules, and even worse translation sources will not be deleted when you remove a module.
Another question: why are translations not cached when a translation is incomplete anyway?
This has significant impact, and only if you really need all settings pages of a site translated does it have a real reason. I would not mind that some untranslated strings are cached if that speeds up my site, as long as the cache get refreshed when I change strings (which would not be a large technical problem to do, and should already work that way)
Comment #30
bwynants CreditAttribution: bwynants commentedapplying the patch gave an amazing increased of speed for my site (speed was on of the reasons not to upgrade my other sites)
If you guys are serious about 4.7 this patch is a must. Or someone needs to rewrite locale.....
Comment #31
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedDries, now that time is on my side I'm willing to update the patch.
Question remains how you want this to be implemented..
Comment #32
smk-ka CreditAttribution: smk-ka commentedOT:
@jadwigo,
@yched:
Just out of interest: could it be possible that you suffer from this bug? Are you using Organic Groups or SMTP module together with localization?
Comment #33
Dries CreditAttribution: Dries commentedPatch no longer applies.
Comment #34
yched CreditAttribution: yched commented@smk-ka : no OG nor SMTP on my side.
I'm not home right now, so I cant't really check for other possible modules triggering the bug you mention
Comment #35
jadwigo CreditAttribution: jadwigo commented@smk-ka
no og.module or smtp.module on my side
Comment #36
Dries CreditAttribution: Dries commentedkilles wrote "IIRC, InnoDB on mysql 3 was not supported.". I can't find proof of that ... do you have a pointer or a reference?
Comment #37
Dries CreditAttribution: Dries commentedJust re-roll the patch. The index is added in a seperate SQL statement so in the event that your database doesn't allow an index to be added, the table would still be created.
Comment #38
meba CreditAttribution: meba commentedThese queries are not using indexes either:
SELECT s.lid, t.translation FROM locales_source s INNER JOIN locales_target t ON s.lid = t.lid WHERE s.source = '' AND t.locale = 'cs'
SELECT locale, name FROM locales_meta WHERE enabled = 1 ORDER BY isdefault DESC, name ASC;
Could anyone update the patch to include indexes for these queries?
Comment #39
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedSince weve decided o drop mysql 3 support, my objections from #28 are no longer valid.
Comment #40
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedSince weve decided o drop mysql 3 support, my objections from #28 are no longer valid.
Comment #41
drummComment #42
Dries CreditAttribution: Dries commentedProbably needs to be re-rolled. Let's get this patch in ...
Comment #43
Heine CreditAttribution: Heine commentedUpdated to apply against HEAD.
Comment #44
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.
Comment #45
(not verified) CreditAttribution: commented