Skip to content

Commit 03fcdfe

Browse files
committed
RequestScopeDeterminator fragt den ScopeMatcher nur noch einmal je Request
Der Dienst fragte bei jedem Aufruf neu, was fuer einen einzigen Speichervorgang rund 6000 Auswertungen derselben, innerhalb einer Anfrage unveraenderlichen Frage bedeutete. Die Antworten werden nun je Request gemerkt: 6137 Aufrufe von ScopeMatcher::isBackendRequest sinken auf 244, gemessen in prod. Die oeffentlichen Signaturen bleiben unveraendert, da die Methoden nicht final sind. Der Request wird nur ueber eine WeakReference gehalten, damit dieser shared Service in langlaufenden Prozessen keinen Request am Leben haelt. Das _scope-Attribut ist Teil des Vergleichs, damit ein erst spaeter vom Router gesetzter Scope die gemerkte Antwort verwirft. Neuer Test deckt beide Verwerfungsfaelle ab.
1 parent e730632 commit 03fcdfe

3 files changed

Lines changed: 316 additions & 6 deletions

File tree

docs/performance-editmask.md

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,8 @@ Webserver tatsächlich geladen hat.
293293
Vorweg: **In Produktion gibt es kein Laufzeitproblem** (746 ms). Alles Folgende ist Kür, kein
294294
Pflichtprogramm — und lohnt nur, wenn es zugleich den Code klarer macht.
295295

296-
1. **`RequestScopeDeterminator` je Request zwischenspeichern.** ~6.000 Auswertungen derselben,
297-
innerhalb einer Anfrage unveränderlichen Frage. Der einzige Fund, der klar nach einem
298-
Fehler aussieht; rund 25 ms.
296+
1. ~~**`RequestScopeDeterminator` je Request zwischenspeichern.**~~ **Umgesetzt**, siehe
297+
[unten](#umgesetzt-der-requestscopedeterminator-merkt-sich-die-antwort).
299298
2. **996 Event-Dispatches je Speichervorgang** — mit 196 ms der größte Einzelposten. Ob das zu
300299
viel ist, ist eine Architekturfrage, keine Optimierungsfrage.
301300
3. **Optimierter Composer-Classmap im Devstack** (`dump-autoload -o`) — Bereitstellung, kein
@@ -334,3 +333,37 @@ Zwei Skripte prüfen die Korrektheit, die bei solchen Umbauten zuerst bricht:
334333
**Beim Messen beachten:** `verify-dnd.js` sortiert `tl_metamodel_dcasetting` per Drag & Drop
335334
um und verändert damit dauerhaft die Reihenfolge der Eingabemaske — nach einem Lauf steht die
336335
Legende woanders. Wer Messreihen fährt, sollte das Skript aus der Runde nehmen.
336+
337+
## Umgesetzt: der RequestScopeDeterminator merkt sich die Antwort
338+
339+
`RequestScopeDeterminator` fragte den Contao-`ScopeMatcher` bei **jedem** Aufruf neu — für
340+
einen einzigen Speichervorgang rund 6.000-mal dieselbe Frage. Die Antwort kann sich innerhalb
341+
einer Anfrage nicht ändern.
342+
343+
Die Klasse merkt sich jetzt je Request, was der Matcher geantwortet hat. Drei Punkte waren
344+
dabei zu beachten:
345+
346+
- **Die Signaturen bleiben unverändert.** `currentScopeIs*()` sind öffentlich und nicht final;
347+
ein nachgerüsteter Rückgabetyp wäre ein BC-Bruch. Die Zwischenspeicherung steckt vollständig
348+
in privaten Feldern.
349+
- **Der Request wird nur schwach gehalten** (`\WeakReference`). Dieser Dienst ist `shared`;
350+
eine starke Referenz würde in langlaufenden Prozessen den letzten Request am Leben halten.
351+
- **Das `_scope`-Attribut gehört zum Vergleich.** Fragt jemand, *bevor* der Router den Scope
352+
gesetzt hat, darf die dann gegebene Antwort später nicht weiterverwendet werden.
353+
354+
Gemessen in prod, verschränkt, sechs Läufe je Variante:
355+
356+
| | Aufrufe `ScopeMatcher::isBackendRequest` | Median |
357+
|---|---:|---:|
358+
| vorher | 6.137 | 786 ms |
359+
| nachher | **244** | 762 ms |
360+
361+
**−96 % Matcher-Aufrufe.** Die 24 ms Wandzeit passen zur Schätzung aus dem Profil, liegen aber
362+
im Rauschen — der belastbare Beleg ist die Aufrufzahl.
363+
364+
Die verbleibenden 244 Aufrufe stammen vom Wechsel zwischen Haupt- und Sub-Request: Der Cache
365+
hat einen Platz, bei Wechsel wird neu ermittelt. Das ist korrekt, nur nicht maximal sparsam;
366+
eine `\WeakMap` je Request käme auf nahezu null, für geschätzte 1 ms Gewinn.
367+
368+
Abgedeckt durch `tests/Contao/RequestScopeDeterminatorTest.php` — insbesondere, dass ein
369+
anderer Request und ein nachträglich gesetzter Scope die gemerkte Antwort verwerfen.

src/Contao/RequestScopeDeterminator.php

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,44 @@ class RequestScopeDeterminator
4646
*/
4747
private RequestStack $requestStack;
4848

49+
/**
50+
* The request the remembered answers below belong to.
51+
*
52+
* Held weakly so that this shared service never keeps a request alive - which would matter in long running
53+
* workers.
54+
*
55+
* @var \WeakReference<Request>|null
56+
*/
57+
private ?\WeakReference $lastRequest = null;
58+
59+
/**
60+
* The `_scope` attribute the remembered answers were determined from, empty string when unset.
61+
*
62+
* @var string
63+
*/
64+
private string $lastScope = '';
65+
66+
/**
67+
* Whether the remembered request is a Contao request, null while undetermined.
68+
*
69+
* @var bool|null
70+
*/
71+
private ?bool $isContao = null;
72+
73+
/**
74+
* Whether the remembered request is a frontend request, null while undetermined.
75+
*
76+
* @var bool|null
77+
*/
78+
private ?bool $isFrontend = null;
79+
80+
/**
81+
* Whether the remembered request is a backend request, null while undetermined.
82+
*
83+
* @var bool|null
84+
*/
85+
private ?bool $isBackend = null;
86+
4987
/**
5088
* Create a new instance.
5189
*
@@ -67,7 +105,12 @@ public function __construct(ScopeMatcher $scopeMatcher, RequestStack $requestSta
67105
*/
68106
public function currentScopeIsUnknown()
69107
{
70-
return (null === ($request = $this->getCurrentRequest())) || !$this->scopeMatcher->isContaoRequest($request);
108+
if (null === ($request = $this->getCurrentRequest())) {
109+
return true;
110+
}
111+
$this->rememberRequest($request);
112+
113+
return !($this->isContao ??= $this->scopeMatcher->isContaoRequest($request));
71114
}
72115

73116
/**
@@ -79,7 +122,12 @@ public function currentScopeIsUnknown()
79122
*/
80123
public function currentScopeIsFrontend()
81124
{
82-
return (null !== ($request = $this->getCurrentRequest())) && $this->scopeMatcher->isFrontendRequest($request);
125+
if (null === ($request = $this->getCurrentRequest())) {
126+
return false;
127+
}
128+
$this->rememberRequest($request);
129+
130+
return $this->isFrontend ??= $this->scopeMatcher->isFrontendRequest($request);
83131
}
84132

85133
/**
@@ -91,7 +139,12 @@ public function currentScopeIsFrontend()
91139
*/
92140
public function currentScopeIsBackend()
93141
{
94-
return (null === ($request = $this->getCurrentRequest())) || $this->scopeMatcher->isBackendRequest($request);
142+
if (null === ($request = $this->getCurrentRequest())) {
143+
return true;
144+
}
145+
$this->rememberRequest($request);
146+
147+
return $this->isBackend ??= $this->scopeMatcher->isBackendRequest($request);
95148
}
96149

97150
/**
@@ -103,4 +156,31 @@ private function getCurrentRequest()
103156
{
104157
return $this->requestStack->getCurrentRequest();
105158
}
159+
160+
/**
161+
* Drop the remembered answers when they do not belong to the passed request any more.
162+
*
163+
* The scope of a request does not change while it is being handled, but this service is asked thousands of times
164+
* per request - saving a single edit mask triggered roughly 6.000 lookups. The `_scope` attribute is part of the
165+
* comparison because it is what the matcher bases its decision on: a request that gets its scope assigned after
166+
* we were first asked - which happens when a listener runs before the router - must not be served a stale answer.
167+
*
168+
* @param Request $request The request the caller is asking about.
169+
*
170+
* @return void
171+
*/
172+
private function rememberRequest(Request $request): void
173+
{
174+
$scope = $request->attributes->getString('_scope');
175+
176+
if ($request === $this->lastRequest?->get() && $scope === $this->lastScope) {
177+
return;
178+
}
179+
180+
$this->lastRequest = \WeakReference::create($request);
181+
$this->lastScope = $scope;
182+
$this->isContao = null;
183+
$this->isFrontend = null;
184+
$this->isBackend = null;
185+
}
106186
}
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
<?php
2+
3+
/**
4+
* This file is part of contao-community-alliance/dc-general.
5+
*
6+
* (c) 2013-2026 Contao Community Alliance.
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*
11+
* This project is provided in good faith and hope to be usable by anyone.
12+
*
13+
* @package contao-community-alliance/dc-general
14+
* @author Ingolf Steinhardt <info@e-spin.de>
15+
* @copyright 2013-2026 Contao Community Alliance.
16+
* @license https://github.com/contao-community-alliance/dc-general/blob/master/LICENSE LGPL-3.0-or-later
17+
* @filesource
18+
*/
19+
20+
declare(strict_types=1);
21+
22+
namespace ContaoCommunityAlliance\DcGeneral\Test\Contao;
23+
24+
use Contao\CoreBundle\Routing\ScopeMatcher;
25+
use ContaoCommunityAlliance\DcGeneral\Contao\RequestScopeDeterminator;
26+
use ContaoCommunityAlliance\DcGeneral\Test\TestCase;
27+
use PHPUnit\Framework\Attributes\CoversClass;
28+
use Symfony\Component\HttpFoundation\Request;
29+
use Symfony\Component\HttpFoundation\RequestStack;
30+
31+
/**
32+
* This tests the request scope determinator.
33+
*/
34+
#[CoversClass(RequestScopeDeterminator::class)]
35+
final class RequestScopeDeterminatorTest extends TestCase
36+
{
37+
/**
38+
* Test that the answer is obtained from the matcher and passed through.
39+
*
40+
* @return void
41+
*/
42+
public function testDeterminesTheScope(): void
43+
{
44+
$request = $this->mockRequest('backend');
45+
$matcher = $this->mockMatcher(backend: true, frontend: false, contao: true);
46+
47+
$determinator = new RequestScopeDeterminator($matcher, $this->mockStack($request));
48+
49+
self::assertTrue($determinator->currentScopeIsBackend());
50+
self::assertFalse($determinator->currentScopeIsFrontend());
51+
self::assertFalse($determinator->currentScopeIsUnknown());
52+
}
53+
54+
/**
55+
* Test that the matcher is consulted only once per request and answer.
56+
*
57+
* @return void
58+
*/
59+
public function testAsksTheMatcherOnlyOnce(): void
60+
{
61+
$request = $this->mockRequest('backend');
62+
63+
$matcher = $this->getMockBuilder(ScopeMatcher::class)->disableOriginalConstructor()->getMock();
64+
$matcher->expects(self::once())->method('isBackendRequest')->with($request)->willReturn(true);
65+
66+
$determinator = new RequestScopeDeterminator($matcher, $this->mockStack($request));
67+
68+
for ($i = 0; $i < 100; $i++) {
69+
self::assertTrue($determinator->currentScopeIsBackend());
70+
}
71+
}
72+
73+
/**
74+
* Test that a different request is determined anew instead of reusing the previous answer.
75+
*
76+
* @return void
77+
*/
78+
public function testDeterminesEachRequestOnItsOwn(): void
79+
{
80+
$backendRequest = $this->mockRequest('backend');
81+
$frontendRequest = $this->mockRequest('frontend');
82+
83+
$stack = $this->createStub(RequestStack::class);
84+
$stack->method('getCurrentRequest')->willReturnOnConsecutiveCalls(
85+
$backendRequest,
86+
$frontendRequest,
87+
$backendRequest
88+
);
89+
90+
$matcher = $this->createStub(ScopeMatcher::class);
91+
$matcher->method('isBackendRequest')->willReturnCallback(
92+
static fn (Request $request): bool => 'backend' === $request->attributes->get('_scope')
93+
);
94+
95+
$determinator = new RequestScopeDeterminator($matcher, $stack);
96+
97+
self::assertTrue($determinator->currentScopeIsBackend());
98+
self::assertFalse($determinator->currentScopeIsBackend());
99+
self::assertTrue($determinator->currentScopeIsBackend());
100+
}
101+
102+
/**
103+
* Test that a scope assigned after the first question invalidates the remembered answer.
104+
*
105+
* This is what happens when something asks before the router has run.
106+
*
107+
* @return void
108+
*/
109+
public function testForgetsTheAnswerWhenTheScopeIsAssignedLater(): void
110+
{
111+
$request = $this->mockRequest(null);
112+
113+
$matcher = $this->createStub(ScopeMatcher::class);
114+
$matcher->method('isBackendRequest')->willReturnCallback(
115+
static fn (Request $request): bool => 'backend' === $request->attributes->get('_scope')
116+
);
117+
118+
$determinator = new RequestScopeDeterminator($matcher, $this->mockStack($request));
119+
120+
self::assertFalse($determinator->currentScopeIsBackend());
121+
122+
$request->attributes->set('_scope', 'backend');
123+
124+
self::assertTrue($determinator->currentScopeIsBackend());
125+
}
126+
127+
/**
128+
* Test that the answers without a request stay as they were.
129+
*
130+
* @return void
131+
*/
132+
public function testAnswersWithoutRequest(): void
133+
{
134+
$stack = $this->createStub(RequestStack::class);
135+
$stack->method('getCurrentRequest')->willReturn(null);
136+
137+
$matcher = $this->getMockBuilder(ScopeMatcher::class)->disableOriginalConstructor()->getMock();
138+
$matcher->expects(self::never())->method('isBackendRequest');
139+
140+
$determinator = new RequestScopeDeterminator($matcher, $stack);
141+
142+
self::assertTrue($determinator->currentScopeIsUnknown());
143+
self::assertTrue($determinator->currentScopeIsBackend());
144+
self::assertFalse($determinator->currentScopeIsFrontend());
145+
}
146+
147+
/**
148+
* Build a request carrying the passed scope.
149+
*
150+
* @param string|null $scope The scope to set, null to leave it unset.
151+
*
152+
* @return Request
153+
*/
154+
private function mockRequest(?string $scope): Request
155+
{
156+
$request = new Request();
157+
if (null !== $scope) {
158+
$request->attributes->set('_scope', $scope);
159+
}
160+
161+
return $request;
162+
}
163+
164+
/**
165+
* Build a request stack always returning the passed request.
166+
*
167+
* @param Request $request The request to return.
168+
*
169+
* @return RequestStack&\PHPUnit\Framework\MockObject\Stub
170+
*/
171+
private function mockStack(Request $request): RequestStack
172+
{
173+
$stack = $this->createStub(RequestStack::class);
174+
$stack->method('getCurrentRequest')->willReturn($request);
175+
176+
return $stack;
177+
}
178+
179+
/**
180+
* Build a scope matcher answering as passed.
181+
*
182+
* @param bool $backend The answer for backend requests.
183+
* @param bool $frontend The answer for frontend requests.
184+
* @param bool $contao The answer for Contao requests.
185+
*
186+
* @return ScopeMatcher&\PHPUnit\Framework\MockObject\Stub
187+
*/
188+
private function mockMatcher(bool $backend, bool $frontend, bool $contao): ScopeMatcher
189+
{
190+
$matcher = $this->createStub(ScopeMatcher::class);
191+
$matcher->method('isBackendRequest')->willReturn($backend);
192+
$matcher->method('isFrontendRequest')->willReturn($frontend);
193+
$matcher->method('isContaoRequest')->willReturn($contao);
194+
195+
return $matcher;
196+
}
197+
}

0 commit comments

Comments
 (0)