EBernhardson (WMF) has submitted this change and it was merged.
Change subject: Remove unnecessary round trips to memcache
......................................................................
Remove unnecessary round trips to memcache
Couple small optimizations that remove trips to memcache when the indexed
values dont change.
Change-Id: Ie4c2d48f4f7a56ccbc87365be575982fa4ad8596
---
M includes/Data/ObjectManager.php
1 file changed, 129 insertions(+), 57 deletions(-)
Approvals:
EBernhardson (WMF): Verified; Looks good to me, approved
diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php
index 08657a7..3d1d999 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -47,6 +47,8 @@
}
// Backing stores, typically in SQL
+// Note that while ObjectLocator implements the above ObjectStorage interface,
ObjectManger
+// cant use this interface because backing stores deal in rows, and OM deals
in objects.
interface WritableObjectStorage extends ObjectStorage {
function insert( array $row );
function update( array $old, array $new );
@@ -164,8 +166,9 @@
}
public function getIterator() {
- throw new \Exception( 'Not Implemented' );
+ return $this->storage->getIterator();
}
+
/**
* All queries must be against the same index. Results are equivilent to
* array_map, maintaining order and key relationship between input
$queries
@@ -293,7 +296,6 @@
/**
* Writable indexes. Error handling is all wrong currently.
- * Cant implement WritableObjectStorage because 'update' method signature
differs
*/
class ObjectManager extends ObjectLocator {
// @var SplObjectStorage $loaded If the object exists then an 'update'
is issued, otherwise 'insert'
@@ -343,13 +345,13 @@
try {
$old = $this->loaded[$object];
$new = $this->mapper->toStorageRow( $object );
+ if ( self::arrayEquals( $old, $new ) ) {
+ return;
+ }
foreach ( $new as $k => $x ) {
if ( $x !== null && !is_scalar( $x ) ) {
throw new \RuntimeException( "Expected
mapper to return all scalars, but '$k' is " . gettype( $x ) );
}
- }
- if ( self::arrayEquals( $old, $new ) ) {
- return;
}
$this->storage->update( $old, $new );
foreach ( $this->lifecycleHandlers as $handler ) {
@@ -358,19 +360,6 @@
$this->loaded[$object] = $new;
} catch ( \Exception $e ) {
throw new PersistenceException( 'failed update', null,
$e );
- }
- }
-
- static public function arrayEquals( array $old, array $new ) {
- return array_diff_assoc( $old, $new ) === array()
- && array_diff_assoc( $new, $old ) === array();
- }
-
- static public function makeArray( $input ) {
- if ( is_array( $input ) ) {
- return $input;
- } else {
- return array( $input );
}
}
@@ -395,6 +384,19 @@
$object = parent::load( $row );
$this->loaded[$object] = $row;
return $object;
+ }
+
+ static public function arrayEquals( array $old, array $new ) {
+ return array_diff_assoc( $old, $new ) === array()
+ && array_diff_assoc( $new, $old ) === array();
+ }
+
+ static public function makeArray( $input ) {
+ if ( is_array( $input ) ) {
+ return $input;
+ } else {
+ return array( $input );
+ }
}
/**
@@ -626,9 +628,9 @@
return $this->indexed;
}
- public function canAnswer( array $keys, array $options ) {
- sort( $keys );
- if ( $keys !== $this->indexedOrdered ) {
+ public function canAnswer( array $featureColumns, array $options ) {
+ sort( $featureColumns );
+ if ( $featureColumns !== $this->indexedOrdered ) {
return false;
}
if ( isset( $options['limit'] ) ) {
@@ -643,29 +645,43 @@
return true;
}
- public function onAfterInsert( $object, array $new) {
+ public function onAfterInsert( $object, array $new ) {
$indexed = ObjectManager::splitFromRow( $new , $this->indexed );
- // is un-indexable a bail-worthy occasion? Probably not
+ // is un-indexable a bail-worthy occasion? Probably not but
makes debugging easier
if ( !$indexed ) {
throw new \MWException( 'Unindexable row: '
.json_encode( $new ) );
}
- $this->addToIndex( $indexed, $new );
+ $compacted = $this->rowCompactor->compactRow( $new );
+ // give implementing index option to create rather than append
+ if ( !$this->maybeCreateIndex( $indexed, $new, $compacted ) ) {
+ // fallback to append
+ $this->addToIndex( $indexed, $compacted );
+ }
}
public function onAfterUpdate( $object, array $old, array $new ) {
$oldIndexed = ObjectManager::splitFromRow( $old, $this->indexed
);
$newIndexed = ObjectManager::splitFromRow( $new, $this->indexed
);
- // TODO: optimize $oldIndexed === $newIndexed, which should be
common
- // TODO: optimize out occurances where data changed but not the
values
- // indexed or stored in the index(for shallow indexes)
if ( !$oldIndexed ) {
throw new \MWException( 'Unindexable row: '
.json_encode( $oldIndexed ) );
}
if ( !$newIndexed ) {
throw new \MWException( 'Unindexable row: '
.json_encode( $newIndexed ) );
}
- $this->removeFromIndex( $oldIndexed, $old );
- $this->addToIndex( $newIndexed, $new );
+ $oldCompacted = $this->rowCompactor->compactRow( $old );
+ $newCompacted = $this->rowCompactor->compactRow( $new );
+ if ( ObjectManager::arrayEquals( $oldIndexed, $newIndexed ) ) {
+ if ( ObjectManager::arrayEquals( $oldCompacted,
$newCompacted ) ) {
+ // Nothing changed in the index
+ return;
+ }
+ // object representation in feature bucket has changed
+ $this->replaceInIndex( $oldIndexed, $oldCompacted,
$newCompacted );
+ } else {
+ // object has moved from one feature bucket to another
+ $this->removeFromIndex( $oldIndexed, $oldCompacted );
+ $this->addToIndex( $newIndexed, $newCompacted );
+ }
}
public function onAfterRemove( $object, array $old ) {
@@ -702,8 +718,10 @@
$key = $this->cacheKey( $query );
// allow for duplicate queries
$keyToIdx[$key][] = $idx;
+ $idxToKey[$idx] = $key;
if ( !isset( $keyToQuery[$key] ) ) {
- $idxToKey[$idx] = $key;
+ // These results will be merged into the query
results, and as such need binary
+ // uuid's as would be received from storage
$keyToQuery[$key] = UUID::convertUUIDs( $query
);
}
}
@@ -748,6 +766,19 @@
return $results;
}
+ // Called prior to self::addToIndex only when new objects as inserted.
Gives the
+ // opportunity for indexes to create rather than append if this object
signifys a new
+ // feature list.
+ protected function maybeCreateIndex( array $indexed, array $sourceRow,
array $compacted ) {
+ return false;
+ }
+
+ // Since these affect the same $indexed bucket implementing classes can
likely
+ // do less round trips than this.
+ protected function replaceInIndex( array $indexed, array $old, array
$new ) {
+ $this->removeFromIndex( $indexed, $old );
+ $this->addToIndex( $indexed, $new );
+ }
protected function cacheKey( array $attributes ) {
global $wgFlowDefaultWikiDb; // meh
@@ -792,12 +823,15 @@
}
protected function addToIndex( array $indexed, array $row ) {
- $row = $this->rowCompactor->compactRow( $row );
$this->cache->set( $this->cacheKey( $indexed ), array( $row ) );
}
protected function removeFromIndex( array $indexed, array $row ) {
$this->cache->delete( $this->cacheKey( $indexed ) );
+ }
+
+ protected function replaceInIndex( array $indexed, array $oldRow, array
$newRow ) {
+ $this->cache->set( $this->cacheKey( $indexed ), array( $newRow
) );
}
}
@@ -827,24 +861,35 @@
}
}
+ public function canAnswer( array $keys, array $options ) {
+ if ( !parent::canAnswer( $keys, $options ) ) {
+ return false;
+ }
+ if ( isset( $options['sort'], $options['order'] ) ) {
+ return $options['sort'] === $this->options['sort']
+ && $options['order'] ===
$this->options['order'];
+ }
+ return true;
+ }
+
public function getLimit() {
return $this->options['limit'];
}
- protected function addToIndex( array $indexed, array $row ) {
- $create = call_user_func( $this->options['create'], $indexed +
$row );
-
- $row = $this->rowCompactor->compactRow( $row );
- $cacheKey = $this->cacheKey( $indexed );
- if ( $create ) {
- $this->cache->set( $cacheKey, array( $row ) );
- return;
+ protected function maybeCreateIndex( array $indexed, array $sourceRow,
array $compacted ) {
+ if ( call_user_func( $this->options['create'], $sourceRow ) ) {
+ $this->cache->set( $this->cacheKey( $indexed ), array(
$compacted ) );
+ return true;
}
+ return false;
+ }
+
+ protected function addToIndex( array $indexed, array $row ) {
$self = $this;
// If this used redis instead of memcached, could it add to
index in position
// without retry possibility? need a single number that will
properly sort rows.
$this->cache->merge(
- $cacheKey,
+ $this->cacheKey( $indexed ),
function( BagOStuff $cache, $key, $value ) use( $self,
$row ) {
if ( $value === false ) {
return false;
@@ -853,16 +898,21 @@
if ( $idx !== false ) {
return false; // This row already
exists somehow
}
- $value[] = $row;
- $value = $self->sortIndex( $value );
- return $self->limitIndexSize( $value );
+ $retval = $value;
+ $retval[] = $row;
+ $retval = $self->sortIndex( $retval );
+ $retval = $self->limitIndexSize( $retval );
+ if ( $retval === $value ) {
+ // object didnt fit in index
+ return false;
+ } else {
+ return $retval;
+ }
}
);
}
protected function removeFromIndex( array $indexed, array $row ) {
- $row = $this->rowCompactor->compactRow( $row );
-
$this->cache->merge(
$this->cacheKey( $indexed ),
function( BagOStuff $cache, $key, $value ) use( $row ) {
@@ -875,6 +925,31 @@
}
unset( $value[$idx] );
return $value;
+ }
+ );
+ }
+
+ protected function replaceInIndex( array $indexed, array $oldRow, array
$newRow ) {
+ $this->cache->merge(
+ $this->cacheKey( $indexed ),
+ function( BagOStuff $cache, $key, $value ) use(
$oldRow, $newRow ) {
+ if ( $value === false ) {
+ return false;
+ }
+ $retval = $value;
+ $idx = array_search( $oldRow, $retval );
+ if ( $idx !== false ) {
+ unset( $retval[$idx] );
+ }
+ $retval[] = $newRow;
+ $retval = $self->sortIndex( $retval );
+ $retval = $self->limitIndexSize( $retval );
+ if ( $value === $retval ) {
+ // new item didnt fit in index and old
item wasnt found in index
+ return false;
+ } else {
+ return $value;
+ }
}
);
}
@@ -907,20 +982,12 @@
return $options;
}
-
- public function canAnswer( array $keys, array $options ) {
- if ( !parent::canAnswer( $keys, $options ) ) {
- return false;
- }
- if ( isset( $options['sort'], $options['order'] ) ) {
- return $options['sort'] === $this->options['sort']
- && $options['order'] ===
$this->options['order'];
- }
- return true;
- }
-
}
+/**
+ * Removes the feature fields from stored array since its duplicating the
cache key values
+ * Re-adds them when retreiving from cache.
+ */
class FeatureCompactor implements Compactor {
public function __construct( array $indexedColumns ) {
$this->indexed = $indexedColumns;
@@ -981,6 +1048,11 @@
}
}
+/**
+ * Backs an index with a UniqueFeatureIndex. This index will store only the
primary key
+ * values from the unique index, and on retrieval from cache will materialize
the primary key
+ * values into full rows from the unique index.
+ */
class ShallowCompactor implements Compactor {
public function __construct( Compactor $inner, UniqueFeatureIndex
$shallow, array $sortedColumns ) {
$this->inner = $inner;
--
To view, visit https://gerrit.wikimedia.org/r/78249
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie4c2d48f4f7a56ccbc87365be575982fa4ad8596
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson (WMF) <ebernhard...@wikimedia.org>
Gerrit-Reviewer: EBernhardson (WMF) <ebernhard...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
