From: Tim Starling <tstarling@wikimedia.org>
Date: Thu, 20 Jun 2019 16:45:19 +0000 (-0500)
Subject: config: ServiceOptions O(1) assert time in the common case
X-Git-Tag: 1.34.0-rc.0~1324^2
X-Git-Url: http://git.heureux-cyclage.org/?a=commitdiff_plain;h=53968321999f3f4264373125c5ab2b61e7f972c1;p=lhc%2Fweb%2Fwiklou.git

config: ServiceOptions O(1) assert time in the common case

Check if the key array passed in the constructor is identical to the key
array passed to assertRequiredOptions(). This takes O(1) time if the key
arrays have the same underlying storage pointer, which is the common
case.

If the arrays have a different order but are otherwise identical, the
slow path is taken instead. The comparison will add O(N) overhead in
addition to the overhead of the array_diff() calls.

Change-Id: Icb9040ab66286b72a270e84f910cb578bed105b0
---

diff --git a/includes/config/ServiceOptions.php b/includes/config/ServiceOptions.php
index 0f3743f794..6ae059ebff 100644
--- a/includes/config/ServiceOptions.php
+++ b/includes/config/ServiceOptions.php
@@ -23,6 +23,7 @@ use Wikimedia\Assert\Assert;
  * @since 1.34
  */
 class ServiceOptions {
+	private $keys = [];
 	private $options = [];
 
 	/**
@@ -33,6 +34,7 @@ class ServiceOptions {
 	 * @throws InvalidArgumentException if one of $keys is not found in any of $sources
 	 */
 	public function __construct( array $keys, ...$sources ) {
+		$this->keys = $keys;
 		foreach ( $keys as $key ) {
 			foreach ( $sources as $source ) {
 				if ( $source instanceof Config ) {
@@ -58,20 +60,21 @@ class ServiceOptions {
 	 * @param string[] $expectedKeys
 	 */
 	public function assertRequiredOptions( array $expectedKeys ) {
-		$actualKeys = array_keys( $this->options );
-		$extraKeys = array_diff( $actualKeys, $expectedKeys );
-		$missingKeys = array_diff( $expectedKeys, $actualKeys );
-		Assert::precondition( !$extraKeys && !$missingKeys,
-			(
-			$extraKeys
-				? 'Unsupported options passed: ' . implode( ', ', $extraKeys ) . '!'
-				: ''
-			) . ( $extraKeys && $missingKeys ? ' ' : '' ) . (
-			$missingKeys
-				? 'Required options missing: ' . implode( ', ', $missingKeys ) . '!'
-				: ''
-			)
-		);
+		if ( $this->keys !== $expectedKeys ) {
+			$extraKeys = array_diff( $this->keys, $expectedKeys );
+			$missingKeys = array_diff( $expectedKeys, $this->keys );
+			Assert::precondition( !$extraKeys && !$missingKeys,
+				(
+				$extraKeys
+					? 'Unsupported options passed: ' . implode( ', ', $extraKeys ) . '!'
+					: ''
+				) . ( $extraKeys && $missingKeys ? ' ' : '' ) . (
+				$missingKeys
+					? 'Required options missing: ' . implode( ', ', $missingKeys ) . '!'
+					: ''
+				)
+			);
+		}
 	}
 
 	/**