From 62d255f698e27f4e76c7fe2c2398ccb516187a9e Mon Sep 17 00:00:00 2001 From: Julien Dehais Date: Fri, 16 Oct 2026 08:58:40 +0200 Subject: [PATCH] perf: read a view's overrides as rows, and rank each one once On a locale with thousands of overrides, the reads behind a catalogue view and the translator's cache miss spent their time outside the database: the grouped read built an entity per override of the browsed locale, and three merges re-ranked both locales in every comparison of their sort. - The grouped read selects the key, the catalogue and the value of each override as a scalar row (findValuesByLocaleAndCatalogues() replaces findByLocaleAndCatalogues(), which only it used). - The effective overrides of a locale and the inherited values are bucketed by precedence instead of sorted. - The translator ranks each row once before sorting. With 5,018 overrides in fr_FR and in fr: the grouped read of 26 catalogues goes from about 225 ms to 80 ms, the effective overrides of seven locales from about 250 ms to 80 ms. The results are unchanged. --- src/Override/OverrideAwareTranslator.php | 6 ++- src/Override/OverrideReader.php | 40 ++++++++++++++----- .../TranslationOverrideRepository.php | 17 ++++---- .../GenerationCostEstimatorTest.php | 7 +++- .../Unit/Override/TranslationManagerTest.php | 22 +++++++--- .../Suggestion/TranslationAiServiceTest.php | 7 +++- 6 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/Override/OverrideAwareTranslator.php b/src/Override/OverrideAwareTranslator.php index 0eeb7c0..c06b09b 100644 --- a/src/Override/OverrideAwareTranslator.php +++ b/src/Override/OverrideAwareTranslator.php @@ -5,6 +5,7 @@ namespace CylleneDigital\AiTranslationBundle\Override; use CylleneDigital\AiTranslationBundle\Catalogue\TranslationCatalogue; +use CylleneDigital\AiTranslationBundle\Entity\TranslationOverride; use CylleneDigital\AiTranslationBundle\Locale\LocaleFallback; use CylleneDigital\AiTranslationBundle\Repository\TranslationOverrideRepository; use CylleneDigital\AiTranslationBundle\Scope\ScopeProviderInterface; @@ -307,7 +308,10 @@ private function fetchOverrides(string $locale, string $scope): array // 3. within a rank, additional-root catalogues ("@label/…" — themes and // friends, whose files also win at runtime) shadow the main root's; // 4. catalogue/key order otherwise. - usort($overrides, static fn ($a, $b): int => [-LocaleFallback::rank($a->getLocale()), '' === $a->getScope(), !str_starts_with($a->getCatalogue(), '@'), $a->getCatalogue(), $a->getKey()] <=> [-LocaleFallback::rank($b->getLocale()), '' === $b->getScope(), !str_starts_with($b->getCatalogue(), '@'), $b->getCatalogue(), $b->getKey()]); + // The sort key of each row is built once: ranking both locales again in every + // comparison weighed more than the query on a large locale. + $sortKeys = array_map(static fn (TranslationOverride $override): array => [-LocaleFallback::rank($override->getLocale()), '' === $override->getScope(), !str_starts_with($override->getCatalogue(), '@'), $override->getCatalogue(), $override->getKey()], $overrides); + uksort($overrides, static fn (int $a, int $b): int => $sortKeys[$a] <=> $sortKeys[$b]); $result = []; /** @var array> $winningLocale domain => key => the locale that answered */ diff --git a/src/Override/OverrideReader.php b/src/Override/OverrideReader.php index 5e4fa68..a5d16d4 100644 --- a/src/Override/OverrideReader.php +++ b/src/Override/OverrideReader.php @@ -67,11 +67,11 @@ public function getTranslationsForCatalogues(array $catalogues, string $locale, } } - foreach ($this->repository->findByLocaleAndCatalogues($locale, $catalogues, $scope) as $override) { - $catalogue = $override->getCatalogue(); - $key = $override->getKey(); + foreach ($this->repository->findValuesByLocaleAndCatalogues($locale, $catalogues, $scope) as $row) { + $catalogue = $row['catalogue']; + $key = $row['key']; $translations[$catalogue][$key] ??= $empty; - $translations[$catalogue][$key]['override'] = $override->getValue(); + $translations[$catalogue][$key]['override'] = $row['value']; $translations[$catalogue][$key]['hasOverride'] = true; } @@ -106,13 +106,21 @@ public function getEffectiveOverridesByCatalogue(string $locale, string $scope = $overrides = $this->repository->findForRuntime($locale, $scope); // Least specific locale first, and within a locale the global rows before the - // scope's own ones: the later passes overwrite. - usort($overrides, static fn (TranslationOverride $a, TranslationOverride $b): int => [LocaleFallback::rank($a->getLocale()), '' === $a->getScope() ? 0 : 1] <=> [LocaleFallback::rank($b->getLocale()), '' === $b->getScope() ? 0 : 1]); + // scope's own ones: the later passes overwrite. Bucketed, not sorted: a comparison + // that ranks both locales again weighed more than the query on a large locale. + $passes = []; + foreach ($overrides as $override) { + $passes[LocaleFallback::rank($override->getLocale()) * 2 + ('' === $override->getScope() ? 0 : 1)][] = $override; + } + + ksort($passes); $byCatalogue = []; - foreach ($overrides as $override) { - $byCatalogue[$override->getCatalogue()][$override->getKey()] = $override->getValue(); + foreach ($passes as $pass) { + foreach ($pass as $override) { + $byCatalogue[$override->getCatalogue()][$override->getKey()] = $override->getValue(); + } } return $byCatalogue; @@ -374,12 +382,22 @@ private function inherited(string $locale, string $scope, ?array $catalogues = n static fn (array $row): bool => $row['locale'] !== $locale || $row['scope'] !== $scope, )); - usort($rows, static fn (array $a, array $b): int => [-LocaleFallback::rank($a['locale']), '' === $a['scope']] <=> [-LocaleFallback::rank($b['locale']), '' === $b['scope']]); + // The more specific locale first, and within a locale the scope's row before the + // global one. Bucketed, not sorted: a comparison that ranks both locales again + // weighed more than the query on a large locale. + $passes = []; + foreach ($rows as $row) { + $passes[-LocaleFallback::rank($row['locale']) * 2 + ('' === $row['scope'] ? 1 : 0)][] = $row; + } + + ksort($passes); $inherited = []; - foreach ($rows as $row) { - $inherited[$row['catalogue']][(string) $row['key']] ??= ['value' => $row['value'], 'locale' => $row['locale'], 'scope' => $row['scope']]; + foreach ($passes as $pass) { + foreach ($pass as $row) { + $inherited[$row['catalogue']][(string) $row['key']] ??= ['value' => $row['value'], 'locale' => $row['locale'], 'scope' => $row['scope']]; + } } return $inherited; diff --git a/src/Repository/TranslationOverrideRepository.php b/src/Repository/TranslationOverrideRepository.php index 6b5d22d..8f86dca 100644 --- a/src/Repository/TranslationOverrideRepository.php +++ b/src/Repository/TranslationOverrideRepository.php @@ -88,20 +88,23 @@ public function lockOneByKey(string $key, string $catalogue, string $locale, str /** * The overrides of these catalogues in one (locale, scope): one query for a whole view, - * filtered in SQL — a single catalogue costs what it always cost. + * filtered in SQL — a single catalogue costs what it always cost. Scalar rows, not + * entities: a view reads thousands of them, and building an entity per row (and + * leaving it in the host's unit of work) cost more than the query itself. * * @param list $catalogues * - * @return list + * @return list */ - public function findByLocaleAndCatalogues(string $locale, array $catalogues, string $scope = ''): array + public function findValuesByLocaleAndCatalogues(string $locale, array $catalogues, string $scope = ''): array { if ([] === $catalogues) { return []; } - /** @var list $result */ - $result = $this->createQueryBuilder('t') + /** @var list $rows */ + $rows = $this->createQueryBuilder('t') + ->select('t.key', 't.catalogue', 't.value') ->andWhere('t.locale = :locale') ->andWhere('t.catalogue IN (:catalogues)') ->andWhere('t.scope = :scope') @@ -111,9 +114,9 @@ public function findByLocaleAndCatalogues(string $locale, array $catalogues, str ->orderBy('t.catalogue', 'ASC') ->addOrderBy('t.key', 'ASC') ->getQuery() - ->getResult(); + ->getArrayResult(); - return $result; + return $rows; } /** diff --git a/tests/Unit/Estimation/GenerationCostEstimatorTest.php b/tests/Unit/Estimation/GenerationCostEstimatorTest.php index 00cf153..1d04b28 100644 --- a/tests/Unit/Estimation/GenerationCostEstimatorTest.php +++ b/tests/Unit/Estimation/GenerationCostEstimatorTest.php @@ -218,8 +218,11 @@ private function createEstimator(array $frMessages, array $frShopMessages = [], $author->method('getAuthorIdentifier')->willReturn(null); $overrideRepository = $this->createStub(TranslationOverrideRepository::class); - $overrideRepository->method('findByLocaleAndCatalogues')->willReturnCallback(static fn (string $locale, array $catalogues, string $scope = ''): array => null !== $overrides - ? array_merge(...array_map(static fn (string $catalogue): array => $overrides($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))) + $overrideRepository->method('findValuesByLocaleAndCatalogues')->willReturnCallback(static fn (string $locale, array $catalogues, string $scope = ''): array => null !== $overrides + ? array_map( + static fn (TranslationOverride $override): array => ['key' => $override->getKey(), 'catalogue' => $override->getCatalogue(), 'value' => $override->getValue()], + array_merge(...array_map(static fn (string $catalogue): array => $overrides($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))), + ) : []); $suggestionRepository = $this->createStub(TranslationSuggestionRepository::class); diff --git a/tests/Unit/Override/TranslationManagerTest.php b/tests/Unit/Override/TranslationManagerTest.php index 4c45b23..40fb751 100644 --- a/tests/Unit/Override/TranslationManagerTest.php +++ b/tests/Unit/Override/TranslationManagerTest.php @@ -89,9 +89,9 @@ public function testTranslationsMergeFileEntriesWithOverrides(): void $orphan->setValue('Orpheline'); $this->repository - ->method('findByLocaleAndCatalogues') + ->method('findValuesByLocaleAndCatalogues') ->with('fr_FR', ['shop/Product/messages']) - ->willReturn([$override, $orphan]); + ->willReturn(self::rows($override, $orphan)); $manager = $this->createManager(); $translations = $manager->getTranslationsForCatalogue('shop/Product/messages', 'fr_FR'); @@ -110,7 +110,7 @@ public function testTranslationsMergeFileEntriesWithOverrides(): void public function testAKeyOnlyTranslatedInAnotherLanguageIsMissing(): void { - $this->repository->method('findByLocaleAndCatalogues')->willReturn([]); + $this->repository->method('findValuesByLocaleAndCatalogues')->willReturn([]); $manager = $this->createManager(); @@ -215,10 +215,10 @@ public function testAScopedBrowseShowsTheGlobalOverrideAsInherited(): void $scoped->setValue('Ajouter au panier (B2B)'); $this->repository - ->method('findByLocaleAndCatalogues') + ->method('findValuesByLocaleAndCatalogues') ->willReturnCallback(static fn (string $locale, array $catalogues, string $scope = ''): array => match ($scope) { - '' => [$global], - 'b2b' => [$scoped], + '' => self::rows($global), + 'b2b' => self::rows($scoped), default => [], }); // What the entry can inherit from — its own b2b row included, set apart by the reader. @@ -312,4 +312,14 @@ private function suggestionRepository(): TranslationSuggestionRepository return $repository; } + + /** + * What the view reads of these overrides: scalar rows. + * + * @return list + */ + private static function rows(TranslationOverride ...$overrides): array + { + return array_map(static fn (TranslationOverride $override): array => ['key' => $override->getKey(), 'catalogue' => $override->getCatalogue(), 'value' => $override->getValue()], array_values($overrides)); + } } diff --git a/tests/Unit/Suggestion/TranslationAiServiceTest.php b/tests/Unit/Suggestion/TranslationAiServiceTest.php index 33d8c20..f577eb7 100644 --- a/tests/Unit/Suggestion/TranslationAiServiceTest.php +++ b/tests/Unit/Suggestion/TranslationAiServiceTest.php @@ -115,8 +115,11 @@ protected function setUp(): void $this->staged[] = $suggestion; }); $this->suggestionRepository->method('findPending')->willReturnCallback(fn (): array => $this->pending); - $this->overrideRepository->method('findByLocaleAndCatalogues')->willReturnCallback(fn (string $locale, array $catalogues, string $scope = ''): array => null !== $this->overrides - ? array_merge(...array_map(fn (string $catalogue): array => ($this->overrides)($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))) + $this->overrideRepository->method('findValuesByLocaleAndCatalogues')->willReturnCallback(fn (string $locale, array $catalogues, string $scope = ''): array => null !== $this->overrides + ? array_map( + static fn (TranslationOverride $override): array => ['key' => $override->getKey(), 'catalogue' => $override->getCatalogue(), 'value' => $override->getValue()], + array_merge(...array_map(fn (string $catalogue): array => ($this->overrides)($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))), + ) : []); // The chain read behind the inherited values, served from the same per-test rows. $this->overrideRepository->method('findInheritable')->willReturnCallback(function (array $locales, string $scope, ?array $catalogues = null): array {