Follow up from: #1064212: Page caching performance has regressed by 30-40%

We need to do the following, some of this might need to be split into sub-issues itself:

1. Do some micro-benchmarks of db_query() vs. db_select() to try to get solid numbers on how much slower db_select() is in terms of building a query.

2. Identify the performance cost of initiating the db_select() class in the first place - does autoloading select.inc and query.inc cost anything vs. just including them? Does including them at all make a difference, both with and without APC?

Files: 

Comments

catch’s picture

StatusFileSize
new1.02 KB
new927 bytes
new841.82 KB
new923.53 KB
new358.5 KB
new505.53 KB

Here's a start.

I created a file that bootstraps to DRUPAL_BOOTSTRAP_DATABASE, starts xhprof profiling, runs SELECT nid FROM node WHERE nid = 1; then stops xhprof profiling.

Also one that runs db_query('SELECT uid FROM {users} WHERE uid = 1')->fetchField(); before profiling to isolate the connection initialization as well.

This gives us more of an idea of what would've caused the regression in #1064212: Page caching performance has regressed by 30-40% since we're including the cost of loading query.inc and select.inc in the profiling - something that you currently don't get from db_query(). It'd be interesting to do a further round of profiling where db_select() has already been loaded once, that would then be a straight fight between the two (assuming you have more than one db_select() during a request, which you might not).

Here's the summary from the second tests with the {users} query run first .

The more fields, conditions etc. in the query, the more I'd expect db_select() to slow down compared to db_query() which is still only dealing with a string, didn't compare this but that's something else again to look at.

db_query():

Total Incl. Wall Time (microsec): 796 microsecs
Total Incl. CPU (microsecs): 0 microsecs
Total Incl. MemUse (bytes): 123,352 bytes
Total Incl. PeakMemUse (bytes): 124,248 bytes
Number of Function Calls: 38

db_select()

Total Incl. Wall Time (microsec): 1,118 microsecs
Total Incl. CPU (microsecs): 0 microsecs
Total Incl. MemUse (bytes): 425,216 bytes
Total Incl. PeakMemUse (bytes): 436,392 bytes
Number of Function Calls: 88

Profiling done with apc enabled, xdebug disabled. I attached full xhprof screenshots of all four runs, and the two scripts (only difference between the two tests is the presence of the {users} query).

catch’s picture

Priority:Major» Normal

That patch is in and there's another issue for clarifying usage, so I downgrading to 'normal'.

andypost’s picture

Suppose db_select() is slower because it's alterable nature - hook_query_*

db_query() does not fire this hooks!

Crell’s picture

db_select() involves 3 times as many function/method calls, including alter hooks. (I think those only fire if there is a tag, actually, so in this case it won't.) A 30% or so increase in runtime is not horrid, but is why we should default to db_query() where possible.

The memory usage I find quite surprising, however. While SelectQuery maintains various arrays internally to build the query, none of them are that big and that's a very simple query. catch, is there some way to try and isolate where that memory is getting used?

catch’s picture

@Crell, I'm not aware of any good tools to isolate exactly where memory gets used. Certainly not in the same way that cpu usage can be isolated.

xhprof can give you an idea where to look, but it does more or less the equivalent of just running memory_get_usage() and memory_get_peak_usage() before and after functions are called as far as I know, so the results are only useful in some cases (one case where they're very useful is looking at the memory usage of the various calls to cache_get() in core - try that on a large D6 or D7 site and be shocked!), unfortunately it's not great at all where the memory is entirely internal stuff in PHP.

I did some work in #664940: investigate general php memory consumption fixes to start identifying exactly where CPU and memory usage comes from when including files, that's very incomplete but I've been intending to get back to it as part of the blog posts I've been doing for D8 performance.

I think we could look at the following to get an idea.

Without APC you'll have CPU/memory overhead from load/parse/compile. However the regression was just as noticeable with APC so we can ignore that difference for the purposes of this IMO.

With or without APC (more or less) you also have the following:
- cost of copying the functions and classes into memory (APC has apc.lazy_classes and apc.lazy_functions but I've yet to get those to work conclusively).
- cost of instantiating the classes.
- stuff that actually happens in the class.

If we look at the first, there are currently ~235 methods defined between select and query.inc, plus variables, I didn't count the classes but it's more than two. I also don't know off the top of my head how many of these get instantiated just from calling db_select(), and how many are used less frequently, however it's clear that InsertQuery, MergeQuery and others won't be needed on a page cache hit.

catch@catch-laptop:~/www/7$ grep -wc "function" includes/database/select.inc
135
catch@catch-laptop:~/www/7$ grep -wc "function" includes/database/query.inc
100

So I think to break this down, we could start to look at the following:

1. Memory used (with and without APC) just from including select.inc
2. Memory used before and after calling db_select($table) - to try to see what the cost of instantiating the class itself is.
3. Not sure where exactly, but try to measure memory usage during the db_select() cycle with both a simple and more complex query.
4. Compare the first three against each other.

This would be useful information to have in general, since understanding what happens here better should inform many of the arguments around code organization, lazy loading, and what to convert to OOP or not during Drupal 8 - at least as far as it impacts performance, and if someone has already done all this work and written it up somewhere I've missed it.

andypost’s picture

@catch I guess this measures should be done for PHP 5.3 ?

I prefer using eAccelerator or ZendServer Data - do you know any statistics about usage comparison APC vs eAccel?

catch’s picture

@andypost yeah I think we should concentrate on 5.3. Loads of sites (in fact every production site I have any involvement with) are on PHP 5.2.x but that's only going to last for so long - and in cases like these, we're not going to be able to optimize this sort of thing for both versions in case there is a difference, so we should do that for the one that is actually supported.

podarok’s picture

subscribe

catch’s picture

Suggestions, would need to profile to see how much difference this makes:

- Move Merge/Insert/Update/Truncate out of query.inc - possibly into their own separate files, or database.write.inc. We can use the existing dynamic require or switch to autoloading for these - it's not worth trying to do minor PHP optimizations on requests that write to the db anyway.

- Consider merging what's left in query.inc (probably not much that much) back into database.inc unless there's a really good reason not to.

- Consider adding a hard require (neither autoload nor dynamic require_once) for select.inc - I believe that will allow APC to cache that class more effectively.

The idea being that we load minimal code (classes/methods/opcode) for requests that only read but do that up front.

Code that is needed for write operations we can then properly lazy load (whether it's real autoload or not).

This may also be a decent test case for apc.lazy_classes and apc.lazy_functions.

Crell’s picture

Do we have any pages that have no write queries? Just session handling is going to have a write query, I think.

One of the catches with the include files is that we currently allow a driver to skip a class if it only needs to use the default implementation. That was intended to improve DX, although it seems to cause a lot of hassle. I'm debating if we want to make the sublcass required, even if it contains nothing, just to simplify the class loading logic. The alternative is to switch the DB layer to use PSR-0-style loading, but that may need to wait for D8/PHP 5.3/namespaces.

That said, are the tests above measuring memory usage from code, or from using db_select? At least right now core always loads all of the DB files unconditionally, so the code weight memory usage will be a wash. I thought the tests above were testing runtime memory usage, not just code load memory usage.

catch’s picture

Sessions are lazy, and session writes themselves are also lazy - only happens if the session contents change iirc so we should have plenty of pages with no writes. The issue that found this was normal page caching and definitely no writes there. The profiling summary is for the entire request, so bootstrap database, run the query, all code loaded and executed in order to do that. The idea when opening this was to isolate running the query from the rest of serving a cached page, I wanted to look at fixed cost per request as well as runtime for this.

catch’s picture

Version:7.x-dev» 8.x-dev
malc0mn’s picture

As this is quite an old issue, I'm wondering if anything more has been investigated about the performance difference? E.g. what is the result with a full page request, not just a single query? As mentioned in #5 There must be a small class loading overhead in here as well?

cafuego’s picture

I've just hit a problem with the use of db_query() all over core when trying to run Drupal on the Drizzle database. Drizzle outright rejects unquoted reserved keywords as column names, such as timestamp.

The database layer has utility functions to properly quote table and field names (on a per-engine basis) but these functions are never used when db_query() is used, so using db_query() prevents the use of additional databases. If db_query() should stay for performance reasons, then Drupal should disallow the use of reserved keywords as table and column names - or at the very least not do so in core.

catch’s picture

@cafuego, this was discussed at length in #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements but that issue is currently won't fix. If there's a problem with this and Drizzle it might be worth bumping #809810: Support the Drizzle database engine, I don't remember the reserved word issue being brought up on there.

edit: I'd also be completely fine with an audit of reserved words in core table/column names and changing those in 8.x.

Jose Reyero’s picture

Just to add some more information, here are my results running a few queries a bit more complex. The second (db_select) is way slower, almost 3x slower!!!!!

- db_query() Tested 6232 strings: 5338 translated, 894 untranslated. Time : 1256.13 ms

- db_select() Tested 6232 strings: 5338 translated, 894 untranslated. Time : 3278.5 ms

And this is how the queries look like:

<?php
   
// With db_query()
   
$translation = db_query("SELECT s.lid, s.version, t.translation FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = :langcode WHERE s.source = :source AND s.context = :context", array(
       
':langcode' => $langcode,
       
':source' => $source,
       
':context' => $context,
    ))->
fetchObject('Drupal\locale\LocaleTranslation');

   
// With db_select()
   
$query = db_select('locales_source', 's');
   
$query->leftJoin('locales_target', 't', 's.lid = t.lid AND t.language = :langcode', array(':langcode' => $langcode));
   
$query
     
->fields('s', array('lid', 'version'))
      ->
fields('t', array('translation', 'customized'))
      ->
condition('s.source', $source)
      ->
condition('s.context', $context);
   
$translation = $query->execute()->fetchObject('Drupal\locale\LocaleTranslation');
?>

Related issue, where the queries come from, etc #1785086: Introduce a generic API for interface translation strings

geerlingguy’s picture

Ah, crud. I take back what I said—was comparing a fetchCol() to a fetchAllAssoc(). Oops. Still slower and uses 5% more RAM, but not as alarming.

geerlingguy’s picture

Issue summary:View changes

I've been tinkering with a sandbox module, DB Perftest, trying to get hard, reproducible numbers to back up the performance-related aspect of this issue, and besides learning how hard it is to get accurate, repeatable memory measurements in PHP (even with XDebug and/or XHProf), I've been able to perform a test run a few thousand times and found that I can get reduce the margin of error to a very insignificant amount for time measurements:

Results for 100 test runs
Array
(
    [db_query() Simple] => 0.431 ms
    [db_select() Simple] => 0.528 ms
    [EntityFieldQuery Simple] => 1.842 ms
    [db_query() with Joins] => 0.384 ms
    [db_select() with Joins] => 0.507 ms
)

The module basically adds an admin page at /admin/config/development/performance/db_perftest, and you can choose which tests to run, and how many times it will run the tests. I'm going to grab a few more 'real-world' queries and also add in some other types of queries to get some helpful performance metrics.

My preliminary findings from just these two small comparisons:

  • For simple queries, db_query() is 22% faster than db_select()
  • For simple queries, db_query() is 124% faster than EFQ
  • For queries with two joins, db_query() is 29% faster than db_select()

Amplify this by (sometimes) hundreds of database queries in a given request, and I'd argue that it's important we don't go overboard with db_select()... I've met many Drupal devs who seem to think that "db_query() is the old Drupal 6 way..." and will *only* use db_select() or EFQ in all code, everywhere.

peterx’s picture

Some sites have many modules using some of the hooks fired by db_select and the overhead of db_select goes up dramatically for small queries. I researched a few cases, not core module cases, and most of them should be replaced with better APIs. Module A asks module B to perform X then uses a query hook to modify a query in B. A better API in module B would avoid the hook.

Which leads to the question - why are modules using the hooks?

I put a watchdog message in db_select on one big site. The site had zero core queries modified. Everything could be converted to db_query. Would be an interesting test across a few sites.