diff --git a/.gitignore b/.gitignore index cd95f32..25b0493 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ composer.lock /node_modules/ .env .php-cs-fixer.cache +.phpbench/ diff --git a/.gitmodules b/.gitmodules index a55a027..ddeb78a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/Engine/EngineTests/EngineTestData"] path = tests/Engine/EngineTests/EngineTestData url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.7.0 + branch = v3.11.0 diff --git a/benchmarks/EngineBench.php b/benchmarks/EngineBench.php new file mode 100644 index 0000000..fca43d7 --- /dev/null +++ b/benchmarks/EngineBench.php @@ -0,0 +1,82 @@ + */ + private array $engineContexts; + + /** @var array */ + private array $flagDependencyContexts; + + private Flagsmith $flagsmith; + + public function setUp(): void + { + $this->engineContexts = array_map( + fn (string $fileName) => self::loadContext(self::TEST_CASES_PATH . "/{$fileName}"), + self::ENGINE_BENCHMARK_TEST_CASES, + ); + $this->flagDependencyContexts = array_map( + fn (string $path) => self::loadContext($path), + glob(self::TEST_CASES_PATH . '/test_flag_dependency__*.jsonc'), + ); + $this->flagsmith = new Flagsmith( + offlineMode: true, + offlineHandler: new LocalFileHandler(__DIR__ . '/../tests/Data/environment.json'), + ); + } + + public function benchEngine(): void + { + foreach ($this->engineContexts as $context) { + Engine::getEvaluationResult($context); + } + } + + public function benchEngineFlagDependency(): void + { + foreach ($this->flagDependencyContexts as $context) { + Engine::getEvaluationResult($context); + } + } + + public function benchLocalEvaluationEnvironmentFlags(): void + { + $this->flagsmith->getEnvironmentFlags(); + } + + public function benchLocalEvaluationIdentityFlags(): void + { + $this->flagsmith->getIdentityFlags('identity', (object) ['foo' => 'bar']); + } + + private static function loadContext(string $path): EvaluationContext + { + return EvaluationContext::fromJsonObject(json5_decode(file_get_contents($path))->context); + } +} diff --git a/composer.json b/composer.json index ebdfe1f..daf22ac 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,8 @@ "symfony/cache": "^5.4.6", "friendsofphp/php-cs-fixer": "^3.6", "doppiogancio/mocked-client": "^3.0", - "colinodell/json5": "^3.0" + "colinodell/json5": "^3.0", + "phpbench/phpbench": "^1.4" }, "autoload": { "psr-4": { diff --git a/phpbench.json b/phpbench.json new file mode 100644 index 0000000..408edc5 --- /dev/null +++ b/phpbench.json @@ -0,0 +1,6 @@ +{ + "$schema": "./vendor/phpbench/phpbench/phpbench.schema.json", + "runner.bootstrap": "vendor/autoload.php", + "runner.path": "benchmarks", + "runner.file_pattern": "*Bench.php" +} diff --git a/src/Engine/Engine.php b/src/Engine/Engine.php index f092b78..85e51c7 100644 --- a/src/Engine/Engine.php +++ b/src/Engine/Engine.php @@ -3,6 +3,7 @@ namespace Flagsmith\Engine; use Flagsmith\Engine\Utils\Hashing; +use Flagsmith\Engine\Utils\LazyFlags; use Flagsmith\Engine\Utils\Semver; use Flagsmith\Engine\Utils\Types\Context\EvaluationContext; use Flagsmith\Engine\Utils\Types\Context\FeatureContext; @@ -46,8 +47,28 @@ public static function getEvaluationResult($context): EvaluationResult $context = self::getEnrichedContext($context); + // Discard any caller-supplied `$.flags`: they are resolved lazily as + // segment conditions read them. Without segments, nothing reads them. + $resolvedFlags = null; + if (!empty($context->segments)) { + $context = clone $context; + $resolvedFlags = new LazyFlags($context); + $context->flags = $resolvedFlags; + } + + $cycleHits = 0; + foreach ($context->segments as $segment) { - if (!self::isContextInSegment($context, $segment)) { + $matches = self::isContextInSegment($context, $segment); + + if ($resolvedFlags->getCycleHits() !== $cycleHits) { + // Evaluating this segment cut a cycle, so it matched, or failed to, + // on a flag that could not be resolved. Neither verdict is sound. + $cycleHits = $resolvedFlags->getCycleHits(); + continue; + } + + if (!$matches) { continue; } @@ -58,14 +79,8 @@ public static function getEvaluationResult($context): EvaluationResult foreach ($segment->overrides as $overrideFeature) { $featureName = $overrideFeature->name; - $evaluatedFeature = $evaluatedFeatures[$featureName] ?? null; - if ($evaluatedFeature) { - $overrideWinsPriority = - ($overrideFeature->priority ?? self::WEAKEST_PRIORITY) < - ($evaluatedFeature->priority ?? self::WEAKEST_PRIORITY); - if (!$overrideWinsPriority) { - continue; - } + if (!self::overrideWins($overrideFeature, $evaluatedFeatures[$featureName] ?? null)) { + continue; } $evaluatedFeatures[$featureName] = $overrideFeature; @@ -89,6 +104,24 @@ public static function getEvaluationResult($context): EvaluationResult ); } + if ($resolvedFlags !== null) { + $context->flags = null; // Frees both by reference counting, not by the cycle collector + } + + if ($resolvedFlags?->isUsed()) { + // Only reached when a segment condition read a flag. Those results take + // precedence: they were resolved with the cycle guard, unlike the + // single-pass recomputation above. + foreach ($resolvedFlags->getResolved() as $featureName => $flag) { + $evaluatedFlags[$featureName] = $flag; + } + foreach ($resolvedFlags->getCyclic() as $featureName) { + if (isset($evaluatedFlags[$featureName])) { + $evaluatedFlags[$featureName]->reason = LazyFlags::CIRCULAR_DEPENDENCY_REASON; + } + } + } + $result = new EvaluationResult(); $result->flags = $evaluatedFlags; $result->segments = $evaluatedSegments; @@ -114,11 +147,29 @@ private static function getEnrichedContext(EvaluationContext $context): Evaluati } /** + * Whether a segment override takes precedence over the one held so far. + * + * Lower priority wins, and the first seen wins a tie, so that precedence + * follows context order rather than the order segments happen to be + * evaluated in. + * @internal + * @param FeatureContext $candidate + * @param ?FeatureContext $incumbent + * @return bool + */ + public static function overrideWins($candidate, $incumbent): bool + { + return $incumbent === null + || ($candidate->priority ?? self::WEAKEST_PRIORITY) < ($incumbent->priority ?? self::WEAKEST_PRIORITY); + } + + /** + * @internal * @param EvaluationContext $context * @param SegmentContext $segment * @return bool */ - private static function isContextInSegment($context, $segment): bool + public static function isContextInSegment($context, $segment): bool { if (empty($segment->rules)) { return false; @@ -134,14 +185,20 @@ private static function isContextInSegment($context, $segment): bool } /** + * @internal * @param FeatureContext $feature * @param ?string $splitKey * @param string $reason * @return FlagResult */ - private static function getFlagResultFromFeatureContext($feature, $splitKey, $reason = 'DEFAULT') + public static function getFlagResultFromFeatureContext($feature, $splitKey, $reason = 'DEFAULT') { + $variantKey = null; + if ($splitKey !== null && !empty($feature->variants)) { + // Default to the control bucket; a matched named variant overrides this. + $variantKey = 'control'; + $hashing = new Hashing(); $percentageValue = $hashing->getHashedPercentageForObjectIds([ $feature->key, @@ -163,6 +220,7 @@ private static function getFlagResultFromFeatureContext($feature, $splitKey, $re $flag->name = $feature->name; $flag->enabled = $feature->enabled; $flag->value = $variant->value; + $flag->variant = $variant->key; $flag->reason = "SPLIT; weight={$variant->weight}"; $flag->metadata = $feature->metadata; return $flag; @@ -175,6 +233,7 @@ private static function getFlagResultFromFeatureContext($feature, $splitKey, $re $flag->name = $feature->name; $flag->enabled = $feature->enabled; $flag->value = $feature->value; + $flag->variant = $variantKey; $flag->reason = $reason; $flag->metadata = $feature->metadata; return $flag; diff --git a/src/Engine/Utils/LazyFlags.php b/src/Engine/Utils/LazyFlags.php new file mode 100644 index 0000000..94822d8 --- /dev/null +++ b/src/Engine/Utils/LazyFlags.php @@ -0,0 +1,214 @@ + + */ +final class LazyFlags implements \ArrayAccess +{ + public const CIRCULAR_DEPENDENCY_REASON = 'ERROR; code=CIRCULAR_DEPENDENCY'; + + /** @var array */ + private array $flags = []; + + /** + * Names of the flags found to be in a dependency cycle. + * @var array + */ + private array $cyclic = []; + + /** + * Set the first time a condition reads a flag. Distinguishes "no + * dependency was ever consulted" from "one was, and resolved to nothing", + * which an empty mapping cannot. + */ + private bool $used = false; + + /** + * Incremented whenever a cycle is cut. A segment whose own evaluation + * increments it read a flag that could not be resolved, so its verdict + * rests on that flag's absence and it must not match. + */ + private int $cycleHits = 0; + + /** + * Stack of the features currently being resolved. + * @var array + */ + private array $resolving = []; + + /** @var array */ + private array $segmentMatches = []; + + /** @var ?array> */ + private ?array $segmentKeysByFeatureName = null; + + public function __construct(private EvaluationContext $context) + { + } + + public function isUsed(): bool + { + return $this->used; + } + + public function getCycleHits(): int + { + return $this->cycleHits; + } + + /** @return array */ + public function getResolved(): array + { + return $this->flags; + } + + /** @return array */ + public function getCyclic(): array + { + return array_keys($this->cyclic); + } + + public function offsetExists(mixed $offset): bool + { + return $this->offsetGet($offset) !== null; + } + + public function offsetGet(mixed $offset): ?FlagResult + { + $featureName = (string) $offset; + if (!array_key_exists($featureName, $this->flags)) { + $this->used = true; + $this->resolveFeature($featureName); + } + return $this->flags[$featureName] ?? null; + } + + public function offsetSet(mixed $offset, mixed $value): void + { + throw new \LogicException('$.flags is read-only'); + } + + public function offsetUnset(mixed $offset): void + { + throw new \LogicException('$.flags is read-only'); + } + + /** + * Resolve the flag for a feature, applying any segment override. + * @param string $featureName + * @return void + */ + private function resolveFeature(string $featureName): void + { + $cycleStart = array_search($featureName, $this->resolving, strict: true); + if ($cycleStart !== false) { + // Cyclic dependency. Leave the flag unresolved. + foreach (array_slice($this->resolving, $cycleStart) as $cyclicFeatureName) { + $this->cyclic[$cyclicFeatureName] = true; + } + $this->cycleHits++; + return; + } + + $feature = $this->context->features[$featureName] ?? null; + if ($feature === null) { + // Depending on a feature absent from the context. + return; + } + + $this->resolving[] = $featureName; + try { + [$overrideFeature, $overrideSegment] = $this->getSegmentOverride($featureName); + } finally { + array_pop($this->resolving); + } + + if (isset($this->cyclic[$featureName])) { + // The result is not something another condition may match on. + return; + } + + $this->flags[$featureName] = $overrideFeature !== null + ? Engine::getFlagResultFromFeatureContext( + $overrideFeature, + $this->context->identity?->key, + "TARGETING_MATCH; segment={$overrideSegment->name}", + ) + : Engine::getFlagResultFromFeatureContext( + $feature, + $this->context->identity?->key, + ); + } + + /** + * @param string $segmentKey + * @return bool + */ + private function matchesSegment(string $segmentKey): bool + { + if (array_key_exists($segmentKey, $this->segmentMatches)) { + return $this->segmentMatches[$segmentKey]; + } + + $cycleHits = $this->cycleHits; + $matches = Engine::isContextInSegment($this->context, $this->context->segments[$segmentKey]); + + if ($this->cycleHits !== $cycleHits) { + // Reached by cutting a cycle, so the verdict rests on a flag that + // could not be resolved. Not a match, and not worth memoising. + return false; + } + + $this->segmentMatches[$segmentKey] = $matches; + return $matches; + } + + /** + * @param string $featureName + * @return array{?FeatureContext, ?SegmentContext} The winning override and its segment + */ + private function getSegmentOverride(string $featureName): array + { + if ($this->segmentKeysByFeatureName === null) { + $this->segmentKeysByFeatureName = []; + foreach ($this->context->segments as $segmentKey => $segment) { + foreach ($segment->overrides as $override) { + $this->segmentKeysByFeatureName[$override->name][] = $segmentKey; + } + } + } + + $overrideFeature = null; + $overrideSegment = null; + foreach ($this->segmentKeysByFeatureName[$featureName] ?? [] as $segmentKey) { + if (!$this->matchesSegment($segmentKey)) { + continue; + } + $segment = $this->context->segments[$segmentKey]; + foreach ($segment->overrides as $override) { + if ($override->name !== $featureName) { + continue; + } + if (Engine::overrideWins($override, $overrideFeature)) { + $overrideFeature = $override; + $overrideSegment = $segment; + } + } + } + + return [$overrideFeature, $overrideSegment]; + } +} diff --git a/src/Engine/Utils/Types/Context/EvaluationContext.php b/src/Engine/Utils/Types/Context/EvaluationContext.php index 9636642..e0b13ea 100644 --- a/src/Engine/Utils/Types/Context/EvaluationContext.php +++ b/src/Engine/Utils/Types/Context/EvaluationContext.php @@ -2,6 +2,8 @@ namespace Flagsmith\Engine\Utils\Types\Context; +use Flagsmith\Engine\Utils\Types\Result\FlagResult; + class EvaluationContext { /** @var EnvironmentContext */ @@ -16,6 +18,13 @@ class EvaluationContext /** @var array */ public $features; + /** + * Read-only: populated by the engine as flags are resolved. Any value + * supplied by the caller is discarded at evaluation time. + * @var null|array|\ArrayAccess + */ + public $flags; + /** * @param object $jsonContext * @return EvaluationContext @@ -112,6 +121,7 @@ private static function _convertFeatures($jsonFeatures): array $feature->variants = []; foreach ($jsonFeature->variants ?? [] as $jsonVariant) { $variant = new FeatureValue(); + $variant->key = $jsonVariant->key ?? null; $variant->value = $jsonVariant->value; $variant->weight = $jsonVariant->weight; $variant->priority = $jsonVariant->priority; diff --git a/src/Engine/Utils/Types/Context/FeatureValue.php b/src/Engine/Utils/Types/Context/FeatureValue.php index 077068d..df39239 100644 --- a/src/Engine/Utils/Types/Context/FeatureValue.php +++ b/src/Engine/Utils/Types/Context/FeatureValue.php @@ -4,6 +4,9 @@ class FeatureValue { + /** @var ?string */ + public $key; + /** @var mixed */ public $value; diff --git a/src/Engine/Utils/Types/Result/FlagResult.php b/src/Engine/Utils/Types/Result/FlagResult.php index f68742b..59c3168 100644 --- a/src/Engine/Utils/Types/Result/FlagResult.php +++ b/src/Engine/Utils/Types/Result/FlagResult.php @@ -13,6 +13,9 @@ class FlagResult implements \JsonSerializable /** @var ?mixed */ public $value; + /** @var ?string */ + public $variant; + /** @var ?string */ public $reason; diff --git a/tests/Engine/EngineTests/EngineTestData b/tests/Engine/EngineTests/EngineTestData index 4b29dc7..28363b3 160000 --- a/tests/Engine/EngineTests/EngineTestData +++ b/tests/Engine/EngineTests/EngineTestData @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit 28363b388de2850c7c3f1b6e7a8bdd41d8da433c