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

objectcache: Avoid getCurrentTime() call in MapCacheLRU::has()

Browse source 
== Background

During a parse request [1] for a popular Wikipedia article, there
are 180,949 calls to MapCacheLRU::has and 188,542 calls to
::getCurrentTime via ::getAge. (Note, getCurrentTime has more calls
than ::has, as there are also calls via ::set and ::setField).

In a one-off XHGui profile, these calls had 246ms wall-time or 258ms
cpu-time attributed to them, which is 0.25s of a 12s parse or 2%.

In a day's worth of index.php samples [2] [3], there is 1.1% cpu-time
or 1.4% wall-time attributed to MapCacheLRU, of which ~0.3% in
getCurrentTime() alone.

== Change

Most uses of MapCacheLRU are without any in-process TTL, and thus have
$maxAge at its default of INF. Avoid calling getAge and getCurrentTime
in that case.

I think long-term it might make sense to deprecate the TTL feature
in MapCacheLRU, as it is very very rarely used, and still adds
significant overhead to all uses MapCacheLRU and its set() code path.
For cases where in-process TTLs are really needed, the HashBagOStuff
class could be used instead. I can't find any complex uses of
get/setFields, let alone with per-sub field TTL, but a theoretical
or third-party use case could potentially use HashBagOStuff with
sub-fields either as their own key, or refactor to put arrays under
a single key and TTL, or re-create it in their own code base if really
needed.

== Difference

I've added a benchmark since generating random values and populating
MapCacheLRU seems non-trivial enough to be worth a re-usable bench.

Using `benchmarkLruHash.php --method get --count 1000`, before:

> MapCacheLRU::get with 500 keys
>    count: 100
>     rate:     28.5/s
>     mean:    35.06ms

... and after:

>   count: 100
>    rate:     35.6/s
>    mean:    28.10ms

[1] POST en.wikipedia.org /w/api.php?action=parse&title=Barack_Obama&text={{:Barack_Obama}}
[2] https://performance.wikimedia.org/arclamp/svgs/daily/2021-09-06.excimer.index.reversed.svgz
[3] https://performance.wikimedia.org/arclamp/svgs/daily/2021-09-06.excimer-wall.index.reversed.svgz

Bug: T275673
Change-Id: Id8c747f0d0a9454274c296090a2ca80e440f97bb
This commit is contained in:
Timo Tijhof 2021-09-07 04:01:31 +01:00 committed by Krinkle
parent 1e781731bc
commit 5873c937d2
2 changed files with 48 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
includes/libs/MapCacheLRU.php
View file

@ -141,11 +141,13 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
*/
public function has( $key, $maxAge = INF ) {
// Optimization: Forego type check because array_key_exists does it already (T275673)
if ( !array_key_exists( $key, $this->cache ) ) {
return false;
}
return ( $maxAge <= 0 || $this->getAge( $key ) <= $maxAge );
return array_key_exists( $key, $this->cache )
&& (
// Optimization: Avoid expensive getAge/getCurrentTime for common case (T275673)
$maxAge === INF
|| $maxAge <= 0
|| $this->getAge( $key ) <= $maxAge
);
}
/**
@ -211,12 +213,14 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
public function hasField( $key, $field, $maxAge = INF ) {
$value = $this->get( $key );
// Optimization: Forego type check because array_key_exists does it already (T275673)
if ( !is_array( $value ) || !array_key_exists( $field, $value ) ) {
return false;
}
return ( $maxAge <= 0 || $this->getAge( $key, $field ) <= $maxAge );
return is_array( $value )
// Optimization: Forego $field type check because array_key_exists does it already (T275673)
&& array_key_exists( $field, $value )
&& (
$maxAge === INF
|| $maxAge <= 0
|| $this->getAge( $key, $field ) <= $maxAge
);
}
/**

36
maintenance/benchmarks/benchmarkLruHash.php
View file

@ -62,13 +62,12 @@ class BenchmarkLruHash extends Benchmarker {
}
if ( !$method || $method === 'set' ) {
// For the set bechmark, do object creation in setup (not measured)
$hObj = null;
$benches['HashBagOStuff::set'] = [
'setup' => static function () use ( &$hObj, $max ) {
$hObj = new HashBagOStuff( [ 'maxKeys' => $max ] );
},
'function' => static function () use ( &$hObj, &$keys ) {
'function' => static function () use ( &$hObj, $keys ) {
foreach ( $keys as $i => $key ) {
$hObj->set( $key, $i );
}
@ -79,7 +78,7 @@ class BenchmarkLruHash extends Benchmarker {
'setup' => static function () use ( &$mObj, $max ) {
$mObj = new MapCacheLRU( $max );
},
'function' => static function () use ( &$mObj, &$keys ) {
'function' => static function () use ( &$mObj, $keys ) {
foreach ( $keys as $i => $key ) {
$mObj->set( $key, $i );
}
@ -87,6 +86,37 @@ class BenchmarkLruHash extends Benchmarker {
];
}
if ( !$method || $method === 'get' ) {
$hObj = null;
$benches['HashBagOStuff::get'] = [
'setup' => static function () use ( &$hObj, $max, $keys ) {
$hObj = new HashBagOStuff( [ 'maxKeys' => $max ] );
foreach ( $keys as $i => $key ) {
$hObj->set( $key, $i );
}
},
'function' => static function () use ( &$hObj, $keys ) {
foreach ( $keys as $key ) {
$hObj->get( $key );
}
}
];
$mObj = null;
$benches['MapCacheLRU::get'] = [
'setup' => static function () use ( &$mObj, $max, $keys ) {
$mObj = new MapCacheLRU( $max );
foreach ( $keys as $i => $key ) {
$mObj->set( $key, $i );
}
},
'function' => static function () use ( &$mObj, $keys ) {
foreach ( $keys as $key ) {
$mObj->get( $key );
}
}
];
}
$this->bench( $benches );
}
}