I was looking in my MySQL logs for queries with no index and I found:
SELECT * FROM nodewords_custom ORDER BY weight ASC
I think an improvement to the query would be to add a WHERE clause to test if the nodewords are active or not on the page:
SELECT * FROM nodewords_custom WHERE enabled = 1 ORDER BY weight ASC
Which would then allow you to add an index to active to reduce the query time when there are many rows in the table.
It would also optimise the PHP as there would be fewer db_fetch_object's to perform when there are pages that are not enabled.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 893762-nodewords-custom-pages-enabled-index.patch | 5.52 KB | dave reid |
Comments
Comment #1
andrewsuth commentedHaving looked into this a little further, it seems the function
_nodewords_get_pages_datais only ever called more than once when the pageadmin/content/nodewords/meta-tags/otheris displayed.I think it would be better to optimise the query for the most common scenario and perhaps have an exception (when loading
../meta-tags/other) to load the whole table.From what I can tell, you only need a query like this for every page load:
About caching the query..
I see it alot in Drupal modules, where developers query the database on every page load. Wouldn't it be more efficient to make the query call once and store it in a global variable, rather than a static one? That way, all subsequent page loads can save on an additional database call?
Comment #2
dave reidAgreed that this needs an index and a more explicit query. Also discovered that the Custom pages overview form is saving non 0/1 values for 'enabled' so it requires an update to fix.
Comment #3
dave reidComment #4
dave reidTested this manually with the upgrade and this helps trim performance on every page. Committed to CVS!
http://drupal.org/cvs?commit=444228
Comment #5
andrewsuth commentedThanks and nice work!