Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

resourceloader: Use FileContentsHasher batching in FileModule::getFileHashes

Browse source 
Instead of hashing each file separately, hash them in a batch.
Previous research on this area of code has identified the suppressing
and restoring of warnings to have a measurable cost, which is why
this was optimised in 3621ad0f82. However, we never really made
use if it (aisde from the 2:1 change in that commit itself), because
we always call it with a single item, turn it into an array, do the
hash and then merge it again.

Instead, we now let it handle a single module's set of files all
at once. Given that this no longer exposes an array of hashes,
also update the (private) signature of getFileHashes to reflect this.

This means all file modules will have their version bumped during the
next MediaWiki release. In general this happens for most releases
and weekly branches already (due to localisation update, general
maintenance, linting changes, and other internal changes). But,
noting here for future reference as this might not be obvious from
the diff.

Change-Id: I4e141a0f5a5c1c1972e2ba33d4b7be6e64ed6ab6
This commit is contained in:
Timo Tijhof 2019-12-15 19:35:20 +00:00 committed by Catrope
parent b18fe40fcc
commit 6bf01cfaa3
2 changed files with 14 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
includes/resourceloader/ResourceLoaderFileModule.php
View file

@ -530,7 +530,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
*
* @see ResourceLoaderModule::getFileDependencies
* @param ResourceLoaderContext $context
* @return array
* @return string
*/
private function getFileHashes( ResourceLoaderContext $context ) {
$files = [];
@ -590,15 +590,11 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
// includes the entry point LESS file that we already have as a master file.
$files = array_unique( $files );
$hashes = [];
foreach ( $files as $file ) {
// Don't include array keys or any other form of file path here, only the hashes.
// Including file paths would needlessly cause global cache invalidation when files
// move on disk or if e.g. the MediaWiki directory name changes.
// Anything where order is significant is already detected by the definition summary.
$hashes[] = ResourceLoaderModule::safeFileHash( $file );
}
return $hashes;
// Don't return array keys or any other form of file path here, only the hashes.
// Including file paths would needlessly cause global cache invalidation when files
// move on disk or if e.g. the MediaWiki directory name changes.
// Anything where order is significant is already detected by the definition summary.
return FileContentsHasher::getFileContentsHash( $files );
}
/**
@ -640,9 +636,9 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
// - The 'files' array, simplied to only which files exist (the keys of
// this array), and something that represents their non-file content.
// For packaged files that reflect files directly from disk, the
// 'getFileHashes' method tracks this already.
// It is important that the keys of the 'files' array are preserved,
// as they affect the module output.
// 'getFileHashes' method tracks their content already.
// It is important that the keys of the $packageFiles['files'] array
// are preserved, as they do affect the module output.
$packageFiles['files'] = array_map( function ( $fileInfo ) {
return $fileInfo['definitionSummary'] ?? ( $fileInfo['content'] ?? null );
}, $packageFiles['files'] );

10
includes/utils/FileContentsHasher.php
View file

@ -48,12 +48,11 @@ class FileContentsHasher {
* Get a hash of a file's contents, either by retrieving a previously-
* computed hash from the cache, or by computing a hash from the file.
*
* @private
* @param string $filePath Full path to the file.
* @param string $algo Name of selected hashing algorithm.
* @return string|bool Hash of file contents, or false if the file could not be read.
*/
public function getFileContentsHashInternal( $filePath, $algo = 'md4' ) {
private function getFileContentsHashInternal( $filePath, $algo = 'md4' ) {
$mtime = filemtime( $filePath );
if ( $mtime === false ) {
return false;
@ -102,9 +101,10 @@ class FileContentsHasher {
}
sort( $filePaths );
$hashes = array_map( function ( $filePath ) use ( $instance, $algo ) {
return $instance->getFileContentsHashInternal( $filePath, $algo ) ?: '';
}, $filePaths );
$hashes = [];
foreach ( $filePaths as $filePath ) {
$hashes[] = $instance->getFileContentsHashInternal( $filePath, $algo ) ?: '';
}
Wikimedia\restoreWarnings();