See http://drupal.org/node/686196#comment-2668760 and downwards for why. I doubt we have time to be doing heavy refactoring of the installer, so we should at least avoid the duplicate work, if not the duplicate function calls.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Subscribing.

carlos8f’s picture

Subscribing, should hopefully be straightforward.

atchijov’s picture

Assigned: Unassigned » atchijov
Status: Active » Needs review
FileSize
539 bytes

Not sure if I have submitted diff in right format. Where can I look for "rules of engagement"?

Status: Needs review » Needs work

The last submitted patch, node-733306-d7.diff, failed testing.

catch’s picture

Roll with cvs diff -up to get it accepted by the bot (or bzr/git equivalent) - full instructions are at http://drupal.org/patch :)

carlos8f’s picture

Assigned: atchijov » Unassigned
Category: bug » task
Status: Needs work » Needs review
FileSize
938 bytes

During the installer this patch prevents about 200 parses with the standard profile, but there are still about 1000+ parses going on, which cannot be statically cached because they are mid-batch (and cache_set() is using FakeCache so that doesn't do any good either). We are parsing the same 100 or so files 10 times each during the batch, but I guess we'll have to re-address for D8.

catch’s picture

#6: 733306-info-file-static-cache.patch queued for re-testing.

catch’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Performance

This makes sense to me, I just profiled a request where drupal_parse_info_file() was being called twice for each theme (due to two calls to _system_rebuild_theme_data()), and parsing 15 .info files twice, took 250ms. So this patch would save around 125ms from that request.

Anonymous’s picture

the base flaw is we don't differentiate between a) "don't use a cache, go directly to the db" and b) "don't use a cache, parse most of the known universe, and the db, and the kitchen sink, because you can just never be too careful".

the module list page is an example where we want b) - if there are new files, we need to see them. for just about everything else, we really only need a).

catch’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7
Dries’s picture

Status: Reviewed & tested by the community » Fixed

This looks good to me too. Committed to 7.x and 8.x.

bfroehle’s picture

Issue tags: -Needs backport to D7

Untagging since it's been committed to 7.x

Status: Fixed » Closed (fixed)
Issue tags: -Performance

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