Page MenuHomePhabricator

Make FileModule version hash deterministic
Closed, ResolvedPublic


As part of T98087, by default modules will use compute their version based on building the module and hashing its produced content.

However for the most common module, FileModule, I estimate this would slow down version computation by at least 10x. This would make If-None-Match/E-Tag handling of module responses too slow. And not to mention the startup module.

This is mainly due to involvement of preprocessors like Lessc that should only be invoked when necessary. While we can't prevent that in a generic way, we can have specific overrides for the FileModule.

Hashing only the files specified in the module is not sufficient because less-files may import other files. And CSS files reference images.

The logic for detecting changes is already in place from the timestamp-based system. Basically, when a module is build, it stores all files it had to access along the way in the database. Then, when computing a version, we check those files in addition to the files currently specified at the top level in the module definition.

It stands to reason that a module can only be different when one of the top level files changed, or when one of the previously known included files changed.

These references being tracked in the database is slightly suboptimal, but has been status quo for a long time and isn't an immediate concern beyond complexity (T90001).

Related Objects

Event Timeline

Krinkle claimed this task.
Krinkle raised the priority of this task from to Medium.
Krinkle updated the task description. (Show Details)
Krinkle added subscribers: ori, Krinkle, Aklapper, Catrope.

I experimented with enabling module content versioning for all modules (except WikiModule, which can't retrieve content, not significant right now).

Locally with latest master, a handful of common extensions installed, warm localisation cache, and mainCache=anything (db). Using $ time curl -i ...load.php?modules=startup&only=scripts&debug=false and XhprofProfiler (StartProfiler: $wgProfiler, ProfilerXhprof, ProfilerOutputDb; view via w/profileinfo.php). Truncate mw.profiling table before the http request.

Before any change, real time is about 1.4s. Sorting profiling data by time%, from 65% and up:

NameTime (%)CountCalls/reqms/callkb/callms/req
ResourceLoaderModule::getVersionHash [+]73.89%2662660.960255.76
ResourceLoader::{array_ma closure} [+]69.89%4460.480241.91
ResourceLoader::getCombinedVersion [+]69.87%22120.930241.86
DatabaseBase::select [+]4.74%25250.87021.79
DatabaseBase::query [+]3.54%26260.63016.31
MessageBlobStore::get [+]2.1%10100.9709.68

After setting enableModuleContentVersion to true in ResourceLoaderModule and ResourceLoaderFileModule the real time from up from 1.4s to 8.5s.

NameTime (%)CountCalls/reqms/callkb/callms/req
ResourceLoaderModule::getModuleContent [+]97.82%221578.1203156.25
ResourceLoaderModule::buildContent [+]97.82%221578.103156.19
lessc::compile [+]55.02%303059.1701775.24
DatabaseBase::select [+]12.6%6326320.640406.59
DatabaseBase::query [+]9.68%6326320.490312.39
MessageBlobStore::get [+]7.72%2622620.950249.08
ResourceLoaderFileModule::getScript [+]5.11%2362360.710167.19

Main hot spots are less compilation (60%), MessageBlobStore (10%), and script/style file reading (20%).

MessageBlobStore still queries about the same number of rows, but is now fetching the blob field (to then sha1) instead of timestamp field. The preloader is currently not populating that so hence the bump there. I imagine without MessageBlobStore it would be much worse (as this store currently saves thousands of memcached and cdb queries, per T90001). Making the preload query include the blobs should help keep the DatabaseBase::select count around 25 instead of going up to 630.

I've also collected some numbers on overhead of computing file hashes vs mtimes.

From using HHVM on mw1017:

PHPHHVM 3.6.1 (srv)
Inputresources/**/* (2241 files)
warmups94.95210647583ms total on average (2 samples)
main81.240646044413ms total on average (30 samples)
warmups126.57392024994ms total on average (2 samples)
main124.19727643331ms total on average (30 samples)
warmups87.63587474823ms total on average (2 samples)
main133.17994276683ms total on average (30 samples)

From localhost using PHP 5.6 on a MacBook Pro:

PHPPHP 5.6.10 (apache2handler)
Inputresources/**/* (2242 files)
warmups92.559576034546ms total on average (2 samples)
main82.975125312805ms total on average (30 samples)
warmups175.01509189606ms total on average (2 samples)
main175.9135723114ms total on average (30 samples)
warmups173.68137836456ms total on average (2 samples)
main165.03329277039ms total on average (30 samples)

Source code of benchmark: P897.

While it hashing does almost twice as long, it's still less than 180ms for all 2000+ files in resources/ (images, less, css, js, i18n json, i18n js). An average module request would iterate over far fewer files. Even the startup module would be limited to a single skin/language combination.

Given these results I think we've got what we need to go ahead. Especially with the net benefit of improved cacheability (less trashing; T102578) this should work out well in the end.

Change 223856 had a related patch set uploaded (by Krinkle):
resourceloader: Convert FileModule to use version hashing

Change 229471 had a related patch set uploaded (by Krinkle):
resourceloader: Convert FileModule to use version hashing

Change 229472 had a related patch set uploaded (by Krinkle):
resourceloader: Convert FileModule to use version hashing

Change 223856 merged by jenkins-bot:
resourceloader: Convert FileModule to use version hashing

Change 229471 merged by jenkins-bot:
resourceloader: Convert FileModule to use version hashing

Change 229472 merged by jenkins-bot:
resourceloader: Convert FileModule to use version hashing