From c2afc7d14f649509e2281b61eb36da309ed02aec Mon Sep 17 00:00:00 2001
From: Lucas Werkmeister <lucas.werkmeister@wikimedia.de>
Date: Mon, 2 Nov 2020 19:02:15 +0100
Subject: [PATCH] Use more iterables in AffectedPagesFinder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

AffectedPagesFinder was written in the hope to be mostly stream-based,
but falls back to arrays in several places. (In the review comments of
change I9ce6a2e903, commit a6701c9e7e, Daniel expressed concerns that
the array approach would not work for entities that are used by millions
of pages, but Q5 is now actually used by almost a million enwiki pages
and I am not aware of any related problems.) However, at least the
helper function transformAllPageEntityUsages() can be turned into a
generator function relatively easily, and this lets us completely avoid
the array roundtrip for non-Item changes – at least as far as
getAffectedPages() goes. (filterUpdates() will still turn the stream
into an array, and ultimately ChangeHandler handles it all at once,
anyways.)

Bug: T266499
Change-Id: I597b8f9747a2dd09f8052b607715ebf3585fa7fb
---
 .../includes/Changes/AffectedPagesFinder.php  | 22 +++++++------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/client/includes/Changes/AffectedPagesFinder.php b/client/includes/Changes/AffectedPagesFinder.php
index 62ffcca8359..39138ee6488 100644
--- a/client/includes/Changes/AffectedPagesFinder.php
+++ b/client/includes/Changes/AffectedPagesFinder.php
@@ -206,8 +206,6 @@ private function getAffectedPages( EntityChange $change ) {
 			array_merge( $changedAspects, [ EntityUsage::ALL_USAGE ] )
 		);
 
-		// @todo: use iterators throughout!
-		$usages = iterator_to_array( $usages, true );
 		$usages = $this->transformAllPageEntityUsages( $usages, $entityId, $changedAspects );
 
 		// if title changed, add virtual usages for both old and new title
@@ -229,17 +227,17 @@ private function getAffectedPages( EntityChange $change ) {
 			$mergedUsages = [];
 			$this->mergeUsagesInto( $usages, $mergedUsages );
 			$this->mergeUsagesInto( $usagesFromDiff, $mergedUsages );
-			$usages = $mergedUsages;
+			$usages = new ArrayIterator( $mergedUsages );
 		}
 
-		return new ArrayIterator( $usages );
+		return $usages;
 	}
 
 	/**
-	 * @param PageEntityUsages[] $from
+	 * @param iterable<PageEntityUsages> $from
 	 * @param PageEntityUsages[] &$into Array to merge into
 	 */
-	private function mergeUsagesInto( array $from, array &$into ) {
+	private function mergeUsagesInto( iterable $from, array &$into ) {
 		foreach ( $from as $pageEntityUsages ) {
 			$key = $pageEntityUsages->getPageId();
 
@@ -357,27 +355,23 @@ private function makeVirtualUsages( array $titles, EntityId $entityId, array $as
 	}
 
 	/**
-	 * @param PageEntityUsages[] $usages
+	 * @param iterable<PageEntityUsages> $usages
 	 * @param EntityId $entityId
 	 * @param string[] $changedAspects
 	 *
-	 * @return PageEntityUsages[]
+	 * @return iterable<PageEntityUsages>
 	 */
-	private function transformAllPageEntityUsages( array $usages, EntityId $entityId, array $changedAspects ) {
+	private function transformAllPageEntityUsages( iterable $usages, EntityId $entityId, array $changedAspects ): iterable {
 		$aspectTransformer = new UsageAspectTransformer();
 		$aspectTransformer->setRelevantAspects( $entityId, $changedAspects );
 
-		$transformed = [];
-
 		foreach ( $usages as $key => $usagesOnPage ) {
 			$transformedUsagesOnPage = $aspectTransformer->transformPageEntityUsages( $usagesOnPage );
 
 			if ( !$transformedUsagesOnPage->isEmpty() ) {
-				$transformed[$key] = $transformedUsagesOnPage;
+				yield $key => $transformedUsagesOnPage;
 			}
 		}
-
-		return $transformed;
 	}
 
 }