In function i18n_string_textgroup_default->load_source(), a query like the following one is generated:

SELECT s.*, i.format AS format, i.objectid AS objectid, i.type AS type, i.property AS property, i.objectindex AS objectindex
FROM locales_source s
LEFT OUTER JOIN i18n_string i ON s.lid = i.lid
WHERE  (s.textgroup = "taxonomy") AND (s.context = "term:316398:description") 
LIMIT 1 OFFSET 0;

There's no index for columns textgroup and context, so a full table scan is required. Adding the index makes a huge difference in performance.

EXPLAIN for the above query without the index:

+----+-------------+-------+--------+---------------+---------+---------+--------------+--------+-------------+
| id | select_type | table | type   | possible_keys | key     | key_len | ref          | rows   | Extra       |
+----+-------------+-------+--------+---------------+---------+---------+--------------+--------+-------------+
|  1 | SIMPLE      | s     | ALL    | NULL          | NULL    | NULL    | NULL         | 524385 | Using where |
|  1 | SIMPLE      | i     | eq_ref | PRIMARY       | PRIMARY | 4       | tekla2.s.lid |      1 |             |
+----+-------------+-------+--------+---------------+---------+---------+--------------+--------+-------------+

After adding the index "alter table locales_source add index tc_idx(textgroup(30), context(30));":

+----+-------------+-------+--------+---------------+---------+---------+--------------+------+-------------+
| id | select_type | table | type   | possible_keys | key     | key_len | ref          | rows | Extra       |
+----+-------------+-------+--------+---------------+---------+---------+--------------+------+-------------+
|  1 | SIMPLE      | s     | ref    | tc_idx        | tc_idx  | 184     | const,const  |    1 | Using where |
|  1 | SIMPLE      | i     | eq_ref | PRIMARY       | PRIMARY | 4       | tekla2.s.lid |    1 |             |
+----+-------------+-------+--------+---------------+---------+---------+--------------+------+-------------+
CommentFileSizeAuthor
#20 i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-20.patch1.33 KBpromes
#5 i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-5.patch1.32 KBMichael Molchanov
#3 i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-3.patch828 bytesmaxmendez

Comments

firebird’s picture

firebird commented

There's another one as well. With a large locales_source table, this index makes the difference between a 1.7 second query and a 0.00 second query. And since these queries are generated by i18n_string_load_multiple(), they see a lot of use.

alter table i18n_string add index tot_idx(type, objectid, textgroup);

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

I can confirm this. It's a killer for large pages. We should consider setting the priority up!

maxmendez’s picture

maxmendez commented
Status: Active » Needs review
StatusFileSize
new i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-3.patch828 bytes

I think this improvement its very important, here are the patch.

jose reyero’s picture

jose reyero commented
Status: Needs review » Needs work

This makes sense. About the patch I think it's missing the 'install' part.

Michael Molchanov’s picture

Michael Molchanov commented
StatusFileSize
new i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-5.patch1.32 KB
Michael Molchanov’s picture

Michael Molchanov commented
Status: Needs work » Needs review
weri’s picture

weri commented
Status: Needs review » Reviewed & tested by the community

I reviewed and tested the patch #5. It works like expected.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Priority: Normal » Critical

this is such an important performance enhancement that I'm increasing the priority of it so that it gets the maintainers attention.

We're using this in production right now, huge performance improvement from this patch on every page load. Please commit asap.

m.lebedev’s picture

m.lebedev commented

Great! This patch solved the problem with the performance on the site. Saving a views occurs for 5 seconds faster. Many thanks!

jose reyero’s picture

jose reyero commented
Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed

Committed, thanks.

PS: Not a good idea "increasing the priority of it so that it gets the maintainers attention.", it may work the other way with the maintainer ignoring your patch because of that, at least that's what I usually do... But it was someone else's patch :p

jose reyero’s picture

jose reyero commented

Any idea about this?
#2414769: Upgrade fails on database upgrade

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Issue tags: +Performance
promes’s picture

promes
Primary language Dutch
commented

The update fails, using MySQL 5.5.30:
The following updates returned messages
i18n_string module
Update #7002

Failed: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was too long; max key length is 1000 bytes: ALTER TABLE {locales_source} ADD INDEX `textgroup_context` (`textgroup`, `context`); Array ( ) in db_add_index() (line 2944 of /xxx/includes/database/database.inc).

It works after I changed
db_add_index('locales_source', 'textgroup_context', array('textgroup', 'context'));
into:
db_add_index('locales_source', 'textgroup_context', array(array('textgroup', 32), 'context'));

promes’s picture

promes
Primary language Dutch
commented
Status: Closed (fixed) » Active
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs work 
+++ b/i18n_string/i18n_string.install
@@ -36,6 +38,8 @@ function i18n_string_install() {
+  db_drop_index('locales_source', 'textgroup_context');

@@ -231,6 +235,14 @@ function i18n_string_update_7001() {
+function i18n_string_update_7002() {
+  db_add_index('locales_source', 'textgroup_context', array('textgroup', 'context'));

Maybe it just needs to drop the index first and re-created it @PROMES?

promes’s picture

promes
Primary language Dutch
commented

Just dropping the index and recreating it without a keylength will not be accepted by MySQL 5.5.30 with error "Specified key was too long; max key length is 1000 bytes".
MySQL requires a keylength in the textgroup. SInce no textgroup key is larger then 32 I used this value.
As I wrote: for me the update works after I changed it: #15.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Ah my bad, can you submit a patch that would resolve this issue for you?

promes’s picture

promes
Primary language Dutch
commented
StatusFileSize
new i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-20.patch1.33 KB

For the update see: #15 and
i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-20.patch

weri’s picture

weri commented
Status: Needs work » Needs review

Set to needs review, that the test-bot grabs the patch.

Status: Needs review » Needs work

The last submitted patch, 20: i18n-Missing_database_index_in_locales_source_for_textgroup_and_context-1940532-20.patch, failed testing.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks a lot for your work! This issue is really important for larger multilang-sites!

promes’s picture

promes
Primary language Dutch
commented
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Needs work » Fixed

The patch in #20 will never work, as it's almost identical to #5, which was committed 2 months before.

Also, the conversation from #15-24 is a duplicate of #2414769: Upgrade fails on database upgrade.

Status: Fixed » Closed (fixed)

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