Coming here from #1014130: install_profile_info() does a file system scan on every request to admin/config (and etc.). _mollom_get_version() also calls install_profile_info(), so on a cache miss it could spend up to 2.5 seconds scanning the file system, whereas we only need to get the cached data from system_get_info().

CommentFileSizeAuthor
#6 mollom-HEAD.module-version.6.patch819 bytessun
#1 mollom-HEAD.profile-version.1.patch973 bytessun
mollom_install_profile_info.patch872 bytescatch

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new mollom-HEAD.profile-version.1.patch973 bytes

Overall, I'm not sure about the importance of this patch... the entire function results are cached, so the code is only executed after flushing all caches.

catch’s picture

catch
he/him
Primary language English
commented

Yes this only runs on a cold start. The way I found this was by profiling the first few clicks in a site installed via an install profile (one that includes mollom). Log in, my account, click admin/config on the toolbar - the admin/config request takes 3-4 seconds of which the file system scan accounts for up to 80% - so while this isn't going to take down anyone's server it does make quite a bit of difference for response times for new admin users.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: -Performance

#1: mollom-HEAD.profile-version.1.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Performance

The last submitted patch, mollom-HEAD.profile-version.1.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Fixed

Thanks for reporting, reviewing, and testing! Committed to HEAD.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Fixed » Needs review
StatusFileSize
new mollom-HEAD.module-version.6.patch819 bytes

Actually... while being there ;)

Status: Needs review » Needs work

The last submitted patch, mollom-HEAD.module-version.6.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Fixed

Also committed that follow-up to HEAD.

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

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

Issue tags: +Performance

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

  • Commit c81e8a4 on master, fai6, 8.x-2.x, fbajs, actions by sun: 
    #1014400 by sun, catch: Fixed file system scan in _mollom_get_version...
  • Commit cbe0a3a on master, fai6, 8.x-2.x, fbajs, actions by sun:
    #1014400 by sun: Updated _mollom_get_version() for system_get_info().

  • Commit c81e8a4 on master, fai6, 8.x-2.x, fbajs, actions by sun: 
    #1014400 by sun, catch: Fixed file system scan in _mollom_get_version...
  • Commit cbe0a3a on master, fai6, 8.x-2.x, fbajs, actions by sun:
    #1014400 by sun: Updated _mollom_get_version() for system_get_info().