From 6bf01cfaa3df56a20b18b675064961c7040a461e Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Sun, 15 Dec 2019 19:35:20 +0000
Subject: [PATCH] resourceloader: Use FileContentsHasher batching in
 FileModule::getFileHashes

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 3621ad0f82f8b6b. 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
---
 .../ResourceLoaderFileModule.php              | 22 ++++++++-----------
 includes/utils/FileContentsHasher.php         | 10 ++++-----
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index bed2d56e125..fe1f27aa4af 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -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'] );
diff --git a/includes/utils/FileContentsHasher.php b/includes/utils/FileContentsHasher.php
index e390f217c74..5343bcb54b5 100644
--- a/includes/utils/FileContentsHasher.php
+++ b/includes/utils/FileContentsHasher.php
@@ -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();