Page MenuHomePhabricator

Try to make wmf-config/wgConf's per-wiki configuration cache redundant
Open, Needs TriagePublic

Description

It's a holdover from the PHP 5 era.

It may still be needed or it may not...

# Try configuration cache

$filename = "/tmp/mw-cache-$wmgVersionNumber/conf-$wgDBname";
if ( defined( 'HHVM_VERSION' ) ) {
	$filename .= '-hhvm';
}

$globals = false;
if ( @filemtime( $filename ) >= filemtime( "$wmfConfigDir/InitialiseSettings.php" ) ) {
	$cacheRecord = @file_get_contents( $filename );
	if ( $cacheRecord !== false ) {
		$globals = unserialize( $cacheRecord );
	}
}

if ( !$globals ) {
	# Get configuration from SiteConfiguration object
	require( "$wmfConfigDir/InitialiseSettings.php" );

	$wikiTags = [];
	foreach ( [ 'private', 'fishbowl', 'special', 'closed', 'flow', 'flaggedrevs', 'small', 'medium',
			'large', 'wikimania', 'wikidata', 'wikidataclient', 'visualeditor-nondefault',
			'commonsuploads', 'nonbetafeatures', 'group0', 'group1', 'group2', 'wikipedia', 'nonglobal',
			'wikitech', 'nonecho', 'mobilemainpagelegacy', 'compact-language-links', 'nowikidatadescriptiontaglines',
			'related-articles-footer-blacklisted-skins',
			'top6-wikipedia'
		] as $tag ) {
		$dblist = MWWikiversions::readDbListFile( $tag );
		if ( in_array( $wgDBname, $dblist ) ) {
			$wikiTags[] = $tag;
		}
	}

	$dbSuffix = ( $site === 'wikipedia' ) ? 'wiki' : $site;
	$confParams = [
		'lang'	=> $lang,
		'docRoot' => $_SERVER['DOCUMENT_ROOT'],
		'site'	=> $site,
		'stdlogo' => "//{$wmfHostnames['upload']}/$site/$lang/b/bc/Wiki.png" ,
	];
	// Add a per-language tag as well
	$wikiTags[] = $wgConf->get( 'wgLanguageCode', $wgDBname, $dbSuffix, $confParams, $wikiTags );
	$globals = $wgConf->getAll( $wgDBname, $dbSuffix, $confParams, $wikiTags );

	# Save cache
	@mkdir( '/tmp/mw-cache-' . $wmgVersionNumber );
	$tmpFile = tempnam( '/tmp/', "conf-$wmgVersionNumber-$wgDBname" );
	if ( $tmpFile && file_put_contents( $tmpFile, serialize( $globals ) ) ) {
		if ( !rename( $tmpFile, $filename ) ) {
			// T136258: Rename failed, cleanup temp file
			unlink( $tmpFile );
		};
	}
}

extract( $globals );

Details

ProjectBranchLines +/-Subject
mediawiki/coremaster+19 -14
mediawiki/coremaster+1 -1
operations/mediawiki-configmaster+30 -52
operations/mediawiki-configmaster+0 -25
operations/mediawiki-configmaster+6 -5
operations/mediawiki-configmaster+1 -1
operations/mediawiki-configmaster+0 -2
operations/mediawiki-configmaster+2 K -29
operations/mediawiki-configmaster+31 -0
operations/mediawiki-configmaster+0 -5
operations/mediawiki-configmaster+4 -11
mediawiki/corewmf/1.35.0-wmf.22+32 -34
mediawiki/coremaster+32 -34
mediawiki/corewmf/1.35.0-wmf.22+85 -61
mediawiki/coremaster+85 -61
operations/mediawiki-configmaster+11 -13
operations/mediawiki-configmaster+12 -11
operations/mediawiki-configmaster+4 -8
mediawiki/coremaster+33 -1
Show related patches Customize query in gerrit

Event Timeline

To be clear: it's not that this doesn't provide us some non-zero performance improvement. I just want to measure what kind of difference it makes in the world of HHVM. If it's needed still, fine. But if it's not, it's a weird legacy thing that I'd love to jettison (as it causes no end of headaches & confusion for people).

Krinkle renamed this task from Investigate whether InitialiseSettings configuration cache is still needed on hhvm to Investigate whether wmf-config's configuration cache is still needed.Dec 10 2019, 10:56 PM
Krinkle edited projects, added Wikimedia-Site-requests; removed HHVM.
Krinkle updated the task description. (Show Details)
Krinkle added a subscriber: Krinkle.

This applies to PHP 7 as well.

Change 576060 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] MWConfigCacheGenerator: Remove unused 'docRoot' wgConf placeholder variable

https://gerrit.wikimedia.org/r/576060

Change 576091 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] multiversion: Optimise readDbListFile() function by 40%

https://gerrit.wikimedia.org/r/576091

It looks like about 1/3rd of the time to generate the config-cache (getMWConfigForCacheing) is spent in MWWikiversions::readDbListFile.

Example profile (link):

  • MWConfigCacheGenerator::getMWConfigForCacheing - 25ms
    • MWWikiversions::readDbListFile, 43 calls, 8.1ms. (0.18ms avg per call)
    • SiteConfiguration::getAll, 1 call, 17.4ms
      • SiteConfiguration::getSetting, 920 calls, 16.2ms (0.02ms avg per call)
    • SiteConfiguration::get, 1 call, 0.04ms

Change 576100 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] Add unit tests for SiteConfiguration extraction with nested strings

https://gerrit.wikimedia.org/r/576100

Change 576100 merged by jenkins-bot:
[mediawiki/core@master] Add unit tests for SiteConfiguration extraction with nested strings

https://gerrit.wikimedia.org/r/576100

Change 576060 merged by jenkins-bot:
[operations/mediawiki-config@master] MWConfigCacheGenerator: Remove unused 'docRoot' wgConf placeholder variable

https://gerrit.wikimedia.org/r/576060

Change 576161 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] SiteConfiguration: Optimise getAll() and doReplace() loops

https://gerrit.wikimedia.org/r/576161

Change 576161 merged by jenkins-bot:
[mediawiki/core@master] SiteConfiguration: Optimise getAll() and doReplace() loops

https://gerrit.wikimedia.org/r/576161

Change 576091 merged by jenkins-bot:
[operations/mediawiki-config@master] multiversion: Optimise readDbListFile() function by 40%

https://gerrit.wikimedia.org/r/576091

Mentioned in SAL (#wikimedia-operations) [2020-03-03T16:34:45Z] <krinkle@deploy1001> Synchronized multiversion/MWWikiversions.php: I8815be28d6a26a1 - T169821 (duration: 01m 04s)

Change 576483 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] SiteConfiguration: Optimise getSetting() internals for getAll()

https://gerrit.wikimedia.org/r/576483

Change 576489 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] tests: Reduce 'family' assertion to just 'wiki-suffix disambig'

https://gerrit.wikimedia.org/r/576489

Change 576490 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] [WIP] MWConfigCacheGenerator: Stop reading most wiki-family dblist files

https://gerrit.wikimedia.org/r/576490

Change 576489 merged by jenkins-bot:
[operations/mediawiki-config@master] tests: Reduce 'family' assertion to just 'wiki-suffix disambig'

https://gerrit.wikimedia.org/r/576489

Change 576483 merged by jenkins-bot:
[mediawiki/core@master] SiteConfiguration: Optimise getSetting() internals for getAll()

https://gerrit.wikimedia.org/r/576483

Change 576864 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@wmf/1.35.0-wmf.22] SiteConfiguration: Optimise getSetting() internals for getAll()

https://gerrit.wikimedia.org/r/576864

Change 576867 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] SiteConfiguration: Optimise arrayMerge()

https://gerrit.wikimedia.org/r/576867

Change 576867 merged by jenkins-bot:
[mediawiki/core@master] SiteConfiguration: Optimise arrayMerge()

https://gerrit.wikimedia.org/r/576867

Change 576490 merged by jenkins-bot:
[operations/mediawiki-config@master] MWConfigCacheGenerator: Stop reading most wiki-family dblist files

https://gerrit.wikimedia.org/r/576490

Change 576864 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.22] SiteConfiguration: Optimise getSetting() internals for getAll()

https://gerrit.wikimedia.org/r/576864

Change 577034 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@wmf/1.35.0-wmf.22] SiteConfiguration: Optimise arrayMerge()

https://gerrit.wikimedia.org/r/577034

Change 577034 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.22] SiteConfiguration: Optimise arrayMerge()

https://gerrit.wikimedia.org/r/577034

The various patches link on this task have all been deployed meanwhile.

Before profile (link, from T169821#5933516):
  • MWConfigCacheGenerator::getMWConfigForCacheing - 25ms
    • MWWikiversions::readDbListFile, 43 calls, 8.1ms. (0.18ms avg per call)
    • SiteConfiguration::getAll, 1 call, 17.4ms
      • SiteConfiguration::getSetting, 920 calls, 16.2ms (0.02ms avg per call)
    • SiteConfiguration::get, 1 call, 0.04ms

After profile (link):

  • MWConfigCacheGenerator::getMWConfigForCacheing - 7ms
    • MWWikiversions::readDbListFile, 35 calls, 2.8ms (0.08ms avg per call)
    • SiteConfiguration::getAll, 1 call, 4.1ms
      • SiteConfiguration::processSetting, 923 calls, 2.9ms (0.003ms avg per call)
    • SiteConfiguration::get, 1 call, 0.02ms

Note that the absolute numbers are somewhat inflated in general here as these come from Tideways/XHGui which adds significant run-time overhead (a fixed cost for each individual function call, and it prevents some opcache optimisations).

Krinkle renamed this task from Investigate whether wmf-config's configuration cache is still needed to Try to make wmf-config/wgConf's per-wiki configuration cache redundant.Mar 5 2020, 9:39 PM

Change 578654 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] tests: Add structure test to disallow loading unused dblists

https://gerrit.wikimedia.org/r/578654

Change 578655 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] MWConfigCacheGenerator: Stop loading five unused dblists on web reqs

https://gerrit.wikimedia.org/r/578655

Change 578655 merged by jenkins-bot:
[operations/mediawiki-config@master] MWConfigCacheGenerator: Stop loading five unused dblists on web reqs

https://gerrit.wikimedia.org/r/578655

Change 578654 merged by jenkins-bot:
[operations/mediawiki-config@master] tests: Add structure test to disallow loading unused dblists

https://gerrit.wikimedia.org/r/578654

First profile - original (link, from T169821#5933516):
  • MWConfigCacheGenerator::getMWConfigForCacheing - 25ms
    • MWWikiversions::readDbListFile, 43 calls, 8.1ms. (0.18ms avg per call)
    • SiteConfiguration::getAll, 1 call, 17.4ms
      • SiteConfiguration::getSetting, 920 calls, 16.2ms (0.02ms avg per call)
    • SiteConfiguration::get, 1 call, 0.04ms
Second profile (link, from T169821#5943818):
  • MWConfigCacheGenerator::getMWConfigForCacheing - 7ms
    • MWWikiversions::readDbListFile, 35 calls, 2.8ms (0.08ms avg per call)
    • SiteConfiguration::getAll, 1 call, 4.1ms
      • SiteConfiguration::processSetting, 923 calls, 2.9ms (0.003ms avg per call)
    • SiteConfiguration::get, 1 call, 0.02ms

Note that the absolute numbers are somewhat inflated as they come from Tideways/XHGui, which adds significant run-time overhead (a fixed cost for each individual function call, and it prevents some opcache optimisations).

A few more optimizations have landed meanwhile. Time for another profile.

Third profile (link):

  • MWConfigCacheGenerator::getMWConfigForCacheing - 4.2ms
    • MWWikiversions::readDbListFile, 26 calls, 1.2ms (0.05ms avg per call)
      • MWWikiversions::evalDbListExpression, 1 call, 0.5ms (This is for `group1.dblist)
        • MWWikiversions::readDbListFile, 4 calls, …
    • SiteConfiguration::getAll, 1 call, 2.8ms
      • SiteConfiguration::processSetting, 923 calls, 2.0ms (0.002ms avg per call)
    • SiteConfiguration::get, 1 call, 0.02ms

Change 579105 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] multiversion: Let buildDBLists.php expand expression dblists

https://gerrit.wikimedia.org/r/579105

Change 579105 merged by jenkins-bot:
[operations/mediawiki-config@master] multiversion: Let buildDBLists.php expand expression dblists

https://gerrit.wikimedia.org/r/579105

Change 579643 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] wgConf: Remove unused 'fullLoadCallback' property assignment

https://gerrit.wikimedia.org/r/579643

Change 579645 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] wgConf: Assign $wgLocalDatabases normally instead of by-ref

https://gerrit.wikimedia.org/r/579645

Change 579651 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] Move $wgConf to CommonSettings.php (1/2)

https://gerrit.wikimedia.org/r/579651

Change 579652 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] Move $wgConf to CommonSettings.php (2/2)

https://gerrit.wikimedia.org/r/579652

Change 579653 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] [WIP] Remove use of the $globals tmp cache file

https://gerrit.wikimedia.org/r/579653

Change 579812 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] SiteConfiguration: Remove by-ref return from getLocalDatabases()

https://gerrit.wikimedia.org/r/579812

Change 579643 merged by jenkins-bot:
[operations/mediawiki-config@master] wgConf: Remove unused 'fullLoadCallback' property assignment

https://gerrit.wikimedia.org/r/579643

Change 579645 merged by jenkins-bot:
[operations/mediawiki-config@master] wgConf: Assign $wgLocalDatabases normally instead of by-ref

https://gerrit.wikimedia.org/r/579645

Change 579651 merged by jenkins-bot:
[operations/mediawiki-config@master] Move $wgConf to CommonSettings.php (1/2)

https://gerrit.wikimedia.org/r/579651

Change 579652 merged by jenkins-bot:
[operations/mediawiki-config@master] Move $wgConf to CommonSettings.php (2/2)

https://gerrit.wikimedia.org/r/579652

Change 579812 merged by jenkins-bot:
[mediawiki/core@master] SiteConfiguration: Remove by-ref return from getLocalDatabases()

https://gerrit.wikimedia.org/r/579812

Today a change to a constant in MediaWiki core (change 684142, modified SCHEMA_COMPAT_WRITE_NEW from 0x10 to 0x100). This caused wmf-config to serve an unexpected combination state where configuration values still had the previous value, due to the config cache having retained the value the constant had when it was stored. The wmf-config cache only considered the mtime of InitialiseSettings.php (and logos.php).

This was fixed by @tstarling in change 704444 by adding another filemtime() check. This should have minimal overhead, but it is still a file operation (with presumably one or more syscalls) in an early and shared code path for all web requests.

When I last measured this in early 2020, it seemed the time to call wgConf was more or less on-par with the code for preparing to and reading from the config cache (2 mtimes, file content read, json parse, and various code in-between). I was already working on removing it based on that, but the added check adds a tiny drop toward the other end of the scale, in case other changes led to the cache being faster again.