[drupal-devel] [feature] Block query consolidation

[m3avrck]

Issue status update for 
http://drupal.org/node/30801
Post a follow up: 
http://drupal.org/project/comments/add/30801

 Project:      Drupal
 Version:      cvs
 Component:    block.module
 Category:     feature requests
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  Allie Micka
 Updated by:   m3avrck
 Status:       patch (ready to be committed)

Exactly running similar to what I was uncovering as well. So seems this
patch is ready to go, time to commit :)




m3avrck



Previous comments:
------------------------------------------------------------------------

Fri, 09 Sep 2005 20:12:03 +0000 : Allie Micka

Attachment: http://drupal.org/files/issues/block_query.patch (1.25 KB)

The blocks module is running a separate query for each region, e.g.:


   SELECT * FROM blocks WHERE theme = 'bluemarine' AND status = 1 AND
region='left' ORDER BY weight, module
   SELECT * FROM blocks WHERE theme = 'bluemarine' AND status = 1 AND
region='right' ORDER BY weight, module
   SELECT * FROM blocks WHERE theme = 'bluemarine' AND status = 1 AND
region='header' ORDER BY weight, module
   SELECT * FROM blocks WHERE theme = 'bluemarine' AND status = 1 AND
region='footer' ORDER BY weight, module


Ultimately, we're going to need all of these so let's save a few
queries.  I have changed the query to:


   SELECT * FROM blocks WHERE theme = 'bluemarine' AND status = 1 ORDER
BY region, weight, module


And set all results to the static $blocks array on the first pass.




------------------------------------------------------------------------

Sun, 11 Sep 2005 23:50:53 +0000 : m3avrck

Attachment: http://drupal.org/files/issues/block.module_5.patch (1.27 KB)

Awesome patch! Noticed the version was set wrong so changed this to CVS.


Also, rerolled the patch, there was an extraneous $region in there no
longer needed. Reducing 4 queries is definetly a great start in
optimizing Drupal :)


Take this patch along with this one (might need to be updated now with
this query rewrite I'm thinking): http://drupal.org/node/27157 ... and
we'll have Drupal's loading of blocks pretty much maxed out with
optimizations, then on to the slooow alias ones... ;)




------------------------------------------------------------------------

Mon, 12 Sep 2005 14:24:56 +0000 : Souvent22

Gotta love when something is just logical. Makes sense, and the patch
worked. I'd say this is ready. Get these blocks sped up a bit. This
coupled with 'block primary key' patch should make the blocks solid.




------------------------------------------------------------------------

Mon, 12 Sep 2005 14:36:00 +0000 : Dries

Has this been benchmarked?  I would expect this to be faster, but it's
good practice nonetheless.  Feel free to throw that other block query
patch into the mix.




------------------------------------------------------------------------

Mon, 12 Sep 2005 14:49:15 +0000 : m3avrck

Dries, I ran some simple benchmarks with the devel.module. Queries with
a default install were reduced from 125 to 121. Query times averaged
around 50ms and this was reduced to roughly 47-48ms. It's not a *huge*
savings, but on a a dev machine, with only one user, this is still good
nonetheless. I imagine on a hard hit Drupal site this would start to
scale up nicely with time/load savings. I'll reroll a patch that
combines the two in a few.




------------------------------------------------------------------------

Mon, 12 Sep 2005 15:26:08 +0000 : Dries

This patch reduces the SQL overhead but adds some CPU-overhead and
memory-overhead.  Hence, it's better to benchmark the number of
requests/second (for example using ab or ab2) ...




------------------------------------------------------------------------

Mon, 12 Sep 2005 17:35:54 +0000 : m3avrck

Attachment: http://drupal.org/files/issues/drupal_14.patch (3.31 KB)

Ok here is an updated patch, along with the blocks table modification of
adding a PRIMARY KEY(module,delta). Spent sometime on #mysql discussing
best practice for this table and adding a primary key is the best bet.
Any indexes would not improve performance since this table will
probably almost never have more than 30 rows in it. The primary key
itself is slightly neglible as well, however, if any query were to
perform advanced queries on this table, the primary key would benefit,
so in best practice, we should have this as well.


I'll try to get some benchmarks up soon backing these simple changes.




------------------------------------------------------------------------

Mon, 12 Sep 2005 17:49:34 +0000 : Allie Micka

This patch will not increase memory overhead unless a theme has one or
more regions that are defined but not in use.  Otherwise, the static
$blocks array will be identical with or without the patch applied.


The patch also does not increase CPU overhead because less code is
processed.  Without the patch, the entire query and parsing routine is
executed 5 or more times  (everything between  if
(!isset($blocks[$region])) { } ).   With the patch, the same code
executes only once.


But the proof is in the pudding.  I'm not sure what your standards are
for ab, but I like to do 1 serial check and 1 check with concurrency so
I can rule out performance gained by increased memory consumption, which
bogs down concurrent requests.


This patch appears to save an average of 9-12ms per request and
increase performance by .05 requests per second.


The relevant bits from my ab results are as follows:


Before Patching
ab -n 100 -c 1 http://localhost/
Requests per second:    2.32 [#/sec] (mean)
Time per request:       430.20 [ms] (mean)


ab -n 100 -c 10 http://localhost/
Requests per second:    2.16 [#/sec] (mean)
Time per request:       4631.00 [ms] (mean)
Time per request:       463.10 [ms] (mean, across all concurrent
requests)


After Patching
ab -n 100 -c 1 http://localhost/
Requests per second:    2.37 [#/sec] (mean)
Time per request:       421.08 [ms] (mean)


ab -n 100 -c 10 http://localhost/
Requests per second:    2.22 [#/sec] (mean)
Time per request:       4510.90 [ms] (mean)
Time per request:       451.09 [ms] (mean, across all concurrent
requests)