Mailing List Archive

jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/405896 )

Change subject: Rearrange UsageDeduplicator implementation
......................................................................


Rearrange UsageDeduplicator implementation

This is basically the exact same implementation, just moved around a
bit. I found the way the code was previously arranged not that optimal.
The code was spread out quite a lot.

Significant changes are:
* The actual deduplication intentionally replaces an array of usages
with a single usage. This makes the later array_walk_recursive run
faster because it does have less 1-element arrays to iterate.
* The actual deduplication use &-references to manipulate the one array,
instead of constantly constructing new ones.
* Structuring is done in one step instead of two. $array[$foo][$bar][]
works just fine here.
* Note that labels and descriptions are not mentioned any more! The same
deduplication logic is applied to all aspects, not only labels and
descriptions. This might be a mistake, but no test fails. If you think
this is a mistake, please add a test that demonstrates why.

Bug: T178079
Change-Id: Ic8ec86088d49b5979a78e99d2cafce5f6c86f94d
---
M client/includes/Usage/UsageDeduplicator.php
1 file changed, 39 insertions(+), 45 deletions(-)

Approvals:
Ladsgroup: Looks good to me, approved
jenkins-bot: Verified



diff --git a/client/includes/Usage/UsageDeduplicator.php b/client/includes/Usage/UsageDeduplicator.php
index 8c7e4ce..8a5fe66 100644
--- a/client/includes/Usage/UsageDeduplicator.php
+++ b/client/includes/Usage/UsageDeduplicator.php
@@ -12,81 +12,75 @@

/**
* @param EntityUsage[] $usages
+ *
* @return EntityUsage[]
*/
public function deduplicate( array $usages ) {
$structuredUsages = $this->structureUsages( $usages );
-
- foreach ( $structuredUsages as $entityId => $usages ) {
- $structuredUsages[$entityId] = $this->deduplicateUsagesPerEntity( $usages );
- }
-
- // Flatten the structured array
- $return = [];
- array_walk_recursive(
- $structuredUsages,
- function ( EntityUsage $usage ) use ( &$return ) {
- $return[$usage->getIdentityString()] = $usage;
- }
- );
- return $return;
+ $structuredUsages = $this->deduplicateStructuredUsages( $structuredUsages );
+ return $this->flattenStructuredUsages( $structuredUsages );
}

/**
* @param EntityUsage[] $usages
- * @return array[]
+ *
+ * @return array[][] three-dimensional array of
+ * [ $entityId => [ $aspectKey => [ EntityUsage $usage, … ], … ], … ]
*/
private function structureUsages( array $usages ) {
$structuredUsages = [];
- foreach ( $usages as $usage ) {
- $entityId = $usage->getEntityId();
- $structuredUsages[$entityId->getSerialization()][] = $usage;
- }

- return array_map( [ $this, 'structureUsagesPerEntity' ], $structuredUsages );
- }
-
- /**
- * @param EntityUsage[] $usages
- * @return array[]
- */
- private function structureUsagesPerEntity( array $usages ) {
- $structuredUsages = [
- EntityUsage::DESCRIPTION_USAGE => [],
- EntityUsage::LABEL_USAGE => [],
- ];
foreach ( $usages as $usage ) {
+ $entityId = $usage->getEntityId()->getSerialization();
$aspect = $usage->getAspect();
- $structuredUsages[$aspect][] = $usage;
+ $structuredUsages[$entityId][$aspect][] = $usage;
}

return $structuredUsages;
}

/**
- * @param array[] $usages
+ * @param array[][] $structuredUsages
+ *
* @return array[]
*/
- private function deduplicateUsagesPerEntity( array $usages ) {
- $usages[EntityUsage::DESCRIPTION_USAGE] = $this->deduplicatePerType(
- $usages[EntityUsage::DESCRIPTION_USAGE]
- );
- $usages[EntityUsage::LABEL_USAGE] = $this->deduplicatePerType(
- $usages[EntityUsage::LABEL_USAGE]
- );
- return $usages;
+ private function deduplicateStructuredUsages( array $structuredUsages ) {
+ foreach ( $structuredUsages as &$usagesPerEntity ) {
+ foreach ( $usagesPerEntity as &$usagesPerAspect ) {
+ $this->deduplicatePerAspect( $usagesPerAspect );
+ }
+ }
+
+ return $structuredUsages;
}

/**
- * @param EntityUsage[] $usages
- * @return EntityUsage[]
+ * @param EntityUsage[] &$usages
*/
- private function deduplicatePerType( array $usages ) {
+ private function deduplicatePerAspect( array &$usages ) {
foreach ( $usages as $usage ) {
if ( $usage->getModifier() === null ) {
- return [ $usage ];
+ // This intentionally flattens the array to a single value
+ $usages = $usage;
+ return;
}
}
+ }
+
+ /**
+ * @param array[] $structuredUsages
+ *
+ * @return EntityUsage[]
+ */
+ private function flattenStructuredUsages( array $structuredUsages ) {
+ $usages = [];
+
+ array_walk_recursive(
+ $structuredUsages,
+ function ( EntityUsage $usage ) use ( &$usages ) {
+ $usages[$usage->getIdentityString()] = $usage;
+ }
+ );

return $usages;
}

--
To view, visit https://gerrit.wikimedia.org/r/405896
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic8ec86088d49b5979a78e99d2cafce5f6c86f94d
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <thiemo.kreuz@wikimedia.de>
Gerrit-Reviewer: Ladsgroup <Ladsgroup@gmail.com>
Gerrit-Reviewer: Thiemo Kreuz (WMDE) <thiemo.kreuz@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits