attached patch cleans up and optimises the registry some more.

1. don't write the registry hook or file caches if nothing has changed - two less db writes for 99% of requests.

2. don't drupal_load modules in bootstrap_invoke_all - its now redundant

3. new function registry_load_path_files --> load the files in the registry cache for earlier (during DRUPAL_BOOTSTRAP_PATH), so we avoid a couple of db lookups in drupal_function_exists calls that happen before the menu system takes over.

4. save a different set of files in registry_cache_path_files for logged in vs anon users. this makes the cache more effective.

CommentFileSizeAuthor
#7 more_faster_5.patch3.81 KBAnonymous (not verified)
#6 more_faster_4.patch3.09 KBAnonymous (not verified)
#4 more_faster_3.patch2.99 KBAnonymous (not verified)
more_faster_2.patch4.14 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

+ $cache_id = 'registry_' . ($user->uid ? 'logged_in:' : 'anon:') . $_GET['q'];

Can we use 'authenticated' and 'anonymous' for consistency?

Also, the code comments need work.

chx’s picture

chx CreditAttribution: chx commented

You are replacing the router item with the actual path. That makes the caching much less efficient -- tons and tons of duplicates.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

just a quick note to say that based on discussions with chx, this patch will get stripped back to just the 'don't write to cache if nothing is changed' part, and we'll open separate issues to tackle the 3. and 4. from the issue description.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs work » Needs review
FileSize
more_faster_3.patch2.99 KB

and here's the patch for 'don't write to cache if nothing is changed'.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Needs work

warning: sort() expects parameter 1 to be array, null given in includes/bootstrap.inc on line 1330.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs work » Needs review
FileSize
more_faster_4.patch3.09 KB

egg, meet face.

catch, thanks for the fast review. here's a non-FAIL patch.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
more_faster_5.patch3.81 KB

more egg, more face. mmmm, egg.

this time with the call to registry_load_path_files.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Reviewed & tested by the community

pre-patch: 4057 passes, 122 fails and 15 exceptions.

post-patch: 4059 passes, 120 fails and 15 exceptions.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs review

cross-posted, #7 gives 4057 passes, 122 fails and 15 exceptions.

I'm wondering why the second eggface patch results in two less simpletest failures...

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

@catch: do you know if its just two change? is it more than two, with some in both directions?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Different results again unpatched... I'm starting to think these are edge cases/environmental conditions in the tests that might be occurring anyway. On one run of each, here's the tests which report differently. Don't have time to do any more in depth comparison.

Patched:
Aggregator
6 test cases complete:
149 passes, 3 fails and 0 exceptions.

Taxonomy
3 test cases complete:
53 passes, 9 fails and 0 exceptions.

Unpatched:
Aggregator
6 test cases complete:
150 passes, 2 fails and 0 exceptions.

Taxonomy
3 test cases complete:
51 passes, 11 fails and 0 exceptions.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Reviewed & tested by the community

@catch: thanks for the info.

yeah, i get different pass/fail numbers depending on how many tests are run, both patched and unpatched.

if i just run agregator and taxonomy, its consistently all passes, patched and unpatched, but running all of the tests leads to similar but often not the same numbers of failures. there be dragons in there somewhere...

setting this back to RTBC, because i think what we have is issues with simpletest rather than this patch.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

I agree with that. SimpleTest failures, though interesting, have nothing to do with this.

Dries’s picture

Dries CreditAttribution: Dries commented

Great. I've committed this to CVS HEAD. Thanks.

BioALIEN’s picture

BioALIEN CreditAttribution: BioALIEN commented
Status: Reviewed & tested by the community » Fixed

As this has been committed, I'm setting it to fixed.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

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