Jeroen De Dauw has submitted this change and it was merged.
Change subject: Use DispatchingEntityIdParser instead of BasicEntityIdParser in
repo
......................................................................
Use DispatchingEntityIdParser instead of BasicEntityIdParser in repo
Constructing a new instance of BasicEntityIdParser, then calling
BasicEntityIdParser::getBuilders each time BasicEntityIdParser::parse()
is called (total of 3314 when purging a medium-size item page)
is costly, in terms of time and memory consumption.
Instead, we should be using DispatchingEntityIdParser and
can use a single instance of it via the WikibaseRepo factory.
Change-Id: I3529daefdec950a4377f503f50f07d53a29ae3b1
---
M repo/includes/SummaryFormatter.php
M repo/includes/WikibaseRepo.php
M repo/includes/content/EntityContent.php
M repo/includes/content/EntityHandler.php
M repo/includes/content/ItemHandler.php
M repo/includes/content/PropertyHandler.php
M repo/includes/rdf/RdfBuilder.php
M repo/includes/store/sql/EntityPerPageTable.php
M repo/includes/store/sql/SqlStore.php
M repo/maintenance/rebuildItemsPerSite.php
M repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
M repo/tests/phpunit/includes/store/StoreTest.php
M repo/tests/phpunit/includes/store/sql/EntityPerPageTableTest.php
M repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
14 files changed, 60 insertions(+), 32 deletions(-)
Approvals:
Jeroen De Dauw: Looks good to me, approved
diff --git a/repo/includes/SummaryFormatter.php
b/repo/includes/SummaryFormatter.php
index 59dab3e..519dfcd 100644
--- a/repo/includes/SummaryFormatter.php
+++ b/repo/includes/SummaryFormatter.php
@@ -8,7 +8,6 @@
use Language;
use MWException;
use ValueFormatters\ValueFormatter;
-use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\EntityIdParsingException;
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index f3b6149..a6667bf 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -450,7 +450,8 @@
public function getStore() {
if ( !$this->store ) {
$this->store = new SqlStore(
- $this->getEntityContentDataCodec()
+ $this->getEntityContentDataCodec(),
+ $this->getEntityIdParser()
);
}
@@ -861,6 +862,7 @@
$codec,
array( $validator ),
$errorLocalizer,
+ $this->getEntityIdParser(),
$siteLinkStore,
$this->getLegacyFormatDetectorCallback()
);
@@ -885,6 +887,7 @@
$codec,
array( $validator ),
$errorLocalizer,
+ $this->getEntityIdParser(),
$propertyInfoStore,
$this->getLegacyFormatDetectorCallback()
);
@@ -925,4 +928,4 @@
);
}
-}
\ No newline at end of file
+}
diff --git a/repo/includes/content/EntityContent.php
b/repo/includes/content/EntityContent.php
index bccbc52..bdbb543 100644
--- a/repo/includes/content/EntityContent.php
+++ b/repo/includes/content/EntityContent.php
@@ -23,7 +23,6 @@
use User;
use ValueFormatters\FormatterOptions;
use ValueFormatters\ValueFormatter;
-use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\Lib\Serializers\SerializationOptions;
@@ -312,19 +311,21 @@
$context->setLanguage( $languageCode );
}
+ $wikibaseRepo = WikibaseRepo::getDefaultInstance();
+
if ( !$uiLanguageFallbackChain ) {
- $factory =
WikibaseRepo::getDefaultInstance()->getLanguageFallbackChainFactory();
+ $factory =
$wikibaseRepo->getLanguageFallbackChainFactory();
$uiLanguageFallbackChain =
$factory->newFromContextForPageView( $context );
}
$formatterOptions->setOption( 'languages',
$uiLanguageFallbackChain );
// get all the necessary services ----
- $snakFormatter =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()
+ $snakFormatter = $wikibaseRepo->getSnakFormatterFactory()
->getSnakFormatter( SnakFormatter::FORMAT_HTML_WIDGET,
$formatterOptions );
- $entityInfoBuilderFactory =
WikibaseRepo::getDefaultInstance()->getStore()->getEntityInfoBuilderFactory();
- $entityContentFactory =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
+ $entityInfoBuilderFactory =
$wikibaseRepo->getStore()->getEntityInfoBuilderFactory();
+ $entityContentFactory =
$wikibaseRepo->getEntityContentFactory();
$serializationOptions = $this->makeSerializationOptions(
$languageCode, $uiLanguageFallbackChain );
@@ -337,17 +338,15 @@
//
---------------------------------------------------------------------
- $idParser = new BasicEntityIdParser();
-
$configBuilder = new ParserOutputJsConfigBuilder(
$entityInfoBuilderFactory,
- $idParser,
+ $wikibaseRepo->getEntityIdParser(),
$entityContentFactory,
new ReferencedEntitiesFinder(),
$context->getLanguage()->getCode()
);
- $dataTypeLookup =
WikibaseRepo::getDefaultInstance()->getPropertyDataTypeLookup();
+ $dataTypeLookup = $wikibaseRepo->getPropertyDataTypeLookup();
return new EntityParserOutputGenerator(
$entityView,
diff --git a/repo/includes/content/EntityHandler.php
b/repo/includes/content/EntityHandler.php
index 9b3d7f2..65459c6 100644
--- a/repo/includes/content/EntityHandler.php
+++ b/repo/includes/content/EntityHandler.php
@@ -18,9 +18,9 @@
use Title;
use User;
use ValueValidators\Result;
-use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\EntityContent;
use Wikibase\Lib\Store\EntityContentDataCodec;
use Wikibase\Lib\Store\EntityRedirect;
@@ -79,6 +79,7 @@
* @param EntityContentDataCodec $contentCodec
* @param EntityValidator[] $preSaveValidators
* @param ValidatorErrorLocalizer $errorLocalizer
+ * @param EntityIdParser $entityIdParser
* @param callable|null $legacyExportFormatDetector Callback to
determine whether a serialized
* blob needs to be re-serialized on export. The callback must
take two parameters,
* the blob an the serialization format. It must return true if
re-serialization is needed.
@@ -93,6 +94,7 @@
EntityContentDataCodec $contentCodec,
array $preSaveValidators,
ValidatorErrorLocalizer $errorLocalizer,
+ EntityIdParser $entityIdParser,
$legacyExportFormatDetector = null
) {
$formats = $contentCodec->getSupportedFormats();
@@ -108,6 +110,7 @@
$this->contentCodec = $contentCodec;
$this->preSaveValidators = $preSaveValidators;
$this->errorLocalizer = $errorLocalizer;
+ $this->entityIdParser = $entityIdParser;
$this->legacyExportFormatDetector = $legacyExportFormatDetector;
}
@@ -399,9 +402,7 @@
* @return EntityId
*/
public function getIdForTitle( Title $target ) {
- $parser = new BasicEntityIdParser();
- $id = $parser->parse( $target->getText() );
- return $id;
+ return $this->entityIdParser->parse( $target->getText() );
}
/**
diff --git a/repo/includes/content/ItemHandler.php
b/repo/includes/content/ItemHandler.php
index acdef3a..25dbf6e 100644
--- a/repo/includes/content/ItemHandler.php
+++ b/repo/includes/content/ItemHandler.php
@@ -5,6 +5,7 @@
use DataUpdate;
use Title;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\EntityContent;
@@ -46,6 +47,7 @@
EntityContentDataCodec $contentCodec,
array $preSaveValidators,
ValidatorErrorLocalizer $errorLocalizer,
+ EntityIdParser $entityIdParser,
SiteLinkCache $siteLinkStore,
$legacyExportFormatDetector = null
) {
@@ -56,6 +58,7 @@
$contentCodec,
$preSaveValidators,
$errorLocalizer,
+ $entityIdParser,
$legacyExportFormatDetector
);
diff --git a/repo/includes/content/PropertyHandler.php
b/repo/includes/content/PropertyHandler.php
index e70a7c5..c4a64f0 100644
--- a/repo/includes/content/PropertyHandler.php
+++ b/repo/includes/content/PropertyHandler.php
@@ -7,6 +7,7 @@
use Title;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\EntityContent;
@@ -52,6 +53,7 @@
* @param EntityContentDataCodec $contentCodec
* @param EntityValidator[] $preSaveValidators
* @param ValidatorErrorLocalizer $errorLocalizer
+ * @param EntityIdParser $entityIdParser
* @param PropertyInfoStore $infoStore
* @param callable|null $legacyExportFormatDetector
*/
@@ -61,6 +63,7 @@
EntityContentDataCodec $contentCodec,
$preSaveValidators,
ValidatorErrorLocalizer $errorLocalizer,
+ EntityIdParser $entityIdParser,
PropertyInfoStore $infoStore,
$legacyExportFormatDetector = null
) {
@@ -71,6 +74,7 @@
$contentCodec,
$preSaveValidators,
$errorLocalizer,
+ $entityIdParser,
$legacyExportFormatDetector
);
diff --git a/repo/includes/rdf/RdfBuilder.php b/repo/includes/rdf/RdfBuilder.php
index fddb209..e966f62 100644
--- a/repo/includes/rdf/RdfBuilder.php
+++ b/repo/includes/rdf/RdfBuilder.php
@@ -471,6 +471,7 @@
* @param EntityLookup $entityLookup
*/
public function resolvedMentionedEntities( EntityLookup $entityLookup )
{
+ // @todo inject a DispatchingEntityIdParser
$idParser = new BasicEntityIdParser();
foreach ( $this->entitiesResolved as $id => $resolved ) {
diff --git a/repo/includes/store/sql/EntityPerPageTable.php
b/repo/includes/store/sql/EntityPerPageTable.php
index ae86eab..5e39f01 100644
--- a/repo/includes/store/sql/EntityPerPageTable.php
+++ b/repo/includes/store/sql/EntityPerPageTable.php
@@ -5,8 +5,8 @@
use DatabaseBase;
use DBError;
use InvalidArgumentException;
-use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\LegacyIdInterpreter;
@@ -26,9 +26,9 @@
class EntityPerPageTable implements EntityPerPage {
/**
- * @var BasicEntityIdParser
+ * @var EntityIdParser
*/
- private $idParser;
+ private $entityIdParser;
/**
* @var bool
@@ -40,14 +40,12 @@
*
* @throws InvalidArgumentException
*/
- public function __construct( $useRedirectTargetColumn = true ) {
+ public function __construct( EntityIdParser $entityIdParser,
$useRedirectTargetColumn = true ) {
if ( !is_bool( $useRedirectTargetColumn ) ) {
throw new InvalidArgumentException(
'$useRedirectTargetColumn must be true or false' );
}
- // FIXME: this needs to be injected if the table is to work
with entities other than items and properties
- $this->idParser = new BasicEntityIdParser();
-
+ $this->entityIdParser = $entityIdParser;
$this->useRedirectTargetColumn = $useRedirectTargetColumn;
}
@@ -464,7 +462,7 @@
return null;
}
- return $this->idParser->parse( $row->epp_redirect_target );
+ return $this->entityIdParser->parse( $row->epp_redirect_target
);
}
}
diff --git a/repo/includes/store/sql/SqlStore.php
b/repo/includes/store/sql/SqlStore.php
index 20d0863..2ac2391 100644
--- a/repo/includes/store/sql/SqlStore.php
+++ b/repo/includes/store/sql/SqlStore.php
@@ -9,6 +9,7 @@
use MWException;
use ObjectCache;
use Revision;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\Lib\Reporting\ObservableMessageReporter;
use Wikibase\Lib\Store\CachingEntityRevisionLookup;
use Wikibase\Lib\Store\EntityContentDataCodec;
@@ -106,6 +107,11 @@
private $contentCodec;
/**
+ * @var EntityIdParser
+ */
+ private $entityIdParser;
+
+ /**
* @var bool
*/
private $useRedirectTargetColumn;
@@ -114,7 +120,8 @@
* @param EntityContentDataCodec $contentCodec
*/
public function __construct(
- EntityContentDataCodec $contentCodec
+ EntityContentDataCodec $contentCodec,
+ EntityIdParser $entityIdParser
) {
$this->contentCodec = $contentCodec;
@@ -124,6 +131,7 @@
$cacheDuration = $settings->getSetting( 'sharedCacheDuration' );
$cacheType = $settings->getSetting( 'sharedCacheType' );
+ $this->entityIdParser = $entityIdParser;
$this->useRedirectTargetColumn = $settings->getSetting(
'useRedirectTargetColumn' );
$this->cachePrefix = $cachePrefix;
@@ -461,7 +469,10 @@
* @return EntityPerPage
*/
public function newEntityPerPage() {
- return new EntityPerPageTable( $this->useRedirectTargetColumn );
+ return new EntityPerPageTable(
+ $this->entityIdParser,
+ $this->useRedirectTargetColumn
+ );
}
/**
diff --git a/repo/maintenance/rebuildItemsPerSite.php
b/repo/maintenance/rebuildItemsPerSite.php
index 9f25119..a4078aa 100644
--- a/repo/maintenance/rebuildItemsPerSite.php
+++ b/repo/maintenance/rebuildItemsPerSite.php
@@ -63,7 +63,7 @@
$useRedirectTargetColumn =
WikibaseRepo::getDefaultInstance()->getSettings()->getSetting(
'useRedirectTargetColumn' );
- $entityPerPage = new EntityPerPageTable(
$useRedirectTargetColumn );
+ $entityPerPage =
WikibaseRepo::getDefaultInstance()->getStore()->newEntityPerPage();
$stream = new EntityPerPageIdPager( $entityPerPage, 'item' );
// Now <s>kill</s> fix the table
diff --git a/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
b/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
index a80c69a..f1b7303 100644
--- a/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
+++ b/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
@@ -5,6 +5,7 @@
use TestSites;
use Title;
use User;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\Repo\Store\SQL\EntityPerPageTable;
@@ -56,7 +57,7 @@
$store = new WikiPageEntityStore(
$wikibaseRepo->getEntityContentFactory(),
$wikibaseRepo->getStore()->newIdGenerator(),
- new EntityPerPageTable()
+ new EntityPerPageTable( new BasicEntityIdParser() )
);
$store->saveEntity( $item, 'UpdateRepoOnMoveJobTest', $user,
EDIT_NEW );
diff --git a/repo/tests/phpunit/includes/store/StoreTest.php
b/repo/tests/phpunit/includes/store/StoreTest.php
index a1d0460..148f2df 100644
--- a/repo/tests/phpunit/includes/store/StoreTest.php
+++ b/repo/tests/phpunit/includes/store/StoreTest.php
@@ -22,10 +22,13 @@
class StoreTest extends \MediaWikiTestCase {
public function instanceProvider() {
- $contentCodec =
WikibaseRepo::getDefaultInstance()->getEntityContentDataCodec();
+ $wikibaseRepo = WikibaseRepo::getDefaultInstance();
$instances = array(
- new SqlStore( $contentCodec )
+ new SqlStore(
+ $wikibaseRepo->getEntityContentDataCodec(),
+ $wikibaseRepo->getEntityIdParser()
+ )
);
return array( $instances );
diff --git a/repo/tests/phpunit/includes/store/sql/EntityPerPageTableTest.php
b/repo/tests/phpunit/includes/store/sql/EntityPerPageTableTest.php
index 44ea1c5..bbacf16 100644
--- a/repo/tests/phpunit/includes/store/sql/EntityPerPageTableTest.php
+++ b/repo/tests/phpunit/includes/store/sql/EntityPerPageTableTest.php
@@ -2,6 +2,7 @@
namespace Wikibase\Test;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
@@ -72,7 +73,9 @@
*/
protected function newEntityPerPageTable( array $entities = array() ) {
$useRedirectTargetColumn =
$this->isRedirectTargetColumnSupported();
- $table = new EntityPerPageTable( $useRedirectTargetColumn );
+ $idParser = new BasicEntityIdParser();
+
+ $table = new EntityPerPageTable( $idParser,
$useRedirectTargetColumn );
$table->clear();
$store = WikibaseRepo::getDefaultInstance()->getEntityStore();
diff --git a/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
b/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
index cfa9fa0..19cf6ec 100644
--- a/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
+++ b/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
@@ -6,6 +6,7 @@
use Revision;
use Status;
use User;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
@@ -37,8 +38,9 @@
class WikiPageEntityStoreTest extends \PHPUnit_Framework_TestCase {
private function newEntityPerPageTable() {
+ $idParser = new BasicEntityIdParser();
$useRedirectTargetColumn =
WikibaseRepo::getDefaultInstance()->getSettings()->getSetting(
'useRedirectTargetColumn' );
- return new EntityPerPageTable( $useRedirectTargetColumn );
+ return new EntityPerPageTable( $idParser,
$useRedirectTargetColumn );
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/167136
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3529daefdec950a4377f503f50f07d53a29ae3b1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
