Conversation
|
Measurement, so it can be repeated or refuted. PHP 8.5.10 CLI, wall clock, one process per run, nothing else running, base Corpus 1 — the manual in this repository, 28 documents. This is what
No measurable difference at that size. A render of 28 documents is dominated by building the service container, and each document carries few enough links that the repetition is small. Corpus 2 — the TYPO3 rendering test, 94 documents. Taken from
Raw seconds, main: 10.099855169 10.119443576 10.132837920 10.249994469 10.253074845 10.428776053 10.442199331 11.228186852 11.434180569 11.765302803 Output. All 142 written files compared between the two arms, identical except for the rendered timestamp, which the corpus prints into three localization pages. Where the time was. From an Xdebug profile of corpus 2 on main: What I measured and dropped. The |
|
A third corpus, and the number that decides where this is worth anything. Corpus 3 — TYPO3CMS-Reference-CoreApi, 1003 documents, theme extension removed from
Raw seconds, main: 202.926980411 213.163388576 216.294648183 All 1192 written files compared between the two arms and identical. That is 5.2 %, against 19.7 % on the 94 document corpus — the gain does not follow the number of documents. Counting
So the beneficiary is a theme that renders a large menu or index into every page, which is what the first corpus does and the third largely does not. Worth stating plainly, because it also bounds what anyone should expect from this. The measurement runs one process per render, wall clock, PHP 8.5.10 CLI, nothing else running, base |
Every internal link goes through AbstractUrlGenerator::generateInternalUrl(), which validates the canonical URL through BaseUri and then computes the path to write. Both steps depend on the canonical URL and on the document being rendered, and a project renders the same site wide menu into every one of its documents, so the same pair is computed again for each of them. The result is now kept for the document currently being rendered and dropped as soon as the output file path or the destination path changes. The repetition it exploits is within one document, so the table is bounded by the distinct link targets of a single document - 187 for the largest document of the project measured below - and does not grow with the number of documents. Both values the shipped subclasses read from the render context are part of the key: RelativeUrlGenerator reads the output file path, AbsoluteUrlGenerator the destination path. The abstract method now states that as the contract, because an implementation reading anything else from the context would not be invalidated. One unit test per value pins it; removing either from the key turns one of them red. Rendering the 94 document TYPO3 rendering test: 10.341 s to 8.300 s, median of 10 runs. Rendering the 28 document manual of this repository: 1.609 s to 1.602 s, that is no measurable change at that size. Output byte identical in both cases apart from the rendered timestamp. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01Dc1FkK1tpzuCUNVkRhxs37 Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
Two things the first commit left open. The two tests that came with it pin the shape of the cache key, and removing the whole memo leaves both of them green - they would pass against main. A third test now counts how often the computation is reached: the same target twice in one document reaches it once, another target reaches it again, and the next document computes the first target anew. Removing the memo, or either half of the key, now turns one of the three red. The result was stored as the return value of the computation, so a subclass that renders something of its own while it computes could have its nested call re-key the table, and the outer result was then written into the next document's table. No implementation in this repository calls back, but packages/guides is published on its own. The store is now skipped when the key moved underneath it, with a test that nests such a call. The contract on the abstract method now also names the way out - override generateInternalUrl() and call the computation directly - and states that whatever else decides the path has to be constant for the whole render, as the link style read by ConfigurableUrlGenerator is. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01Dc1FkK1tpzuCUNVkRhxs37 Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
4d342ec to
5f2cff5
Compare
|
|
||
| if ($this->internalUrlCacheKey !== $cacheKey) { | ||
| $this->internalUrlCache = []; | ||
| $this->internalUrlCacheKey = $cacheKey; |
There was a problem hiding this comment.
Why is this property needed? as far as I can see this is only used in the scope of this method. So $cacheKey can be used? This reduced the internal state of this class and possible conflicts.
I think when you store the cache based on the $cachekey you are already safe and there is no need to reset the cache. But maybe i'm overlooking something
There was a problem hiding this comment.
For memory saving, in regards to your comment in #1287
|
Closing tis in favor of #1405 |
2 participants
Merging this makes every internal link that a document renders more than once cost an array lookup instead of a
BaseUriparse plus a path computation. How much that is worth depends on how often a document repeats a link target, not on how large the project is: a 94 document project whose pages repeat 78 % of their links renders in 8.300 s instead of 10.341 s, a 1003 document project that repeats 8 % of them in 202.1 s instead of 213.2 s, and this repository's own 28 document manual shows no measurable change at all. The rendered output is identical in every case.Why
AbstractUrlGenerator::generateInternalUrl()validates the canonical URL throughBaseUri::from()and then hands it togenerateInternalPathFromRelativeUrl(). Both steps depend on the canonical URL and on the document being rendered, and a theme that renders a site wide menu into every page asks for the same targets once per page. In a profile of the 94 document render,League\Uriconstruction and parsing alone accounts for roughly 15 % of the recorded cost, from 128,359 URI objects.The result is now kept for the document currently being rendered and dropped as soon as the output file path or the destination path changes.
The memory question
@jaapio, this is the point you raised on #1287 — an extra table is only worth it if it stays small. This one is held for one document and dropped when the next begins, so its upper bound is the distinct link targets of a single document: 187 for the largest page of the 94 document project, against 94 documents rendered. It does not grow with the size of the project. Interleaving two documents costs the speedup, never the correctness, because a changed output path always invalidates.
This replaces the
AbstractUrlGeneratorpart of #1287, including the still unboundedrelativeUrlCachethat the rework there left in place. If you take this, I will reduce #1287 to what is left of it, or close it.Contract, and what I did not build
Both values the shipped subclasses read from the render context are in the key:
RelativeUrlGeneratorreads the output file path,AbsoluteUrlGeneratorthe destination path.ConfigurableUrlGeneratordelegates to those two and additionally reads the link style, which is fixed when the container is compiled.ExternalUrlGeneratordoes not extend this class. The abstract method states this as its contract and names the way out for an implementation that cannot hold to it: overridegenerateInternalUrl()and call the computation directly.I considered making the memo opt-in through a marker interface, the way you proposed
PreNodeRendererCachableSupportson #1287, so that a subclass outside this repository would not get it. I did not, because the risk is hypothetical here: there is noextends AbstractUrlGeneratoroutsidepackages/, and an instrumented run that recomputes every cache hit and compares found 0 mismatches over 2282 calls across all three test suites. Say the word and I will add it.Tests
Three unit tests, one per thing that can break. Two pin the halves of the key: a relative URL has to follow the document being rendered, an absolute one the destination path. The third counts how often the computation is reached, because a memo is only visible through the calls it avoids — without it, deleting the entire cache left the first two green. A fourth pins a subclass whose computation renders another document, which must not leave its result in the table the nested call re-keyed.
Each of those four defects was injected and produced exactly one red test; the tree is green with none of them.
No phpbench case: the repetition this exploits is across a render, not within one call, and
phpbench.jsonpoints atguides-restructured-textonly. The measurements are full renders, with the commands to repeat them in the comment below.Gates
phpunitunit 610, functional 120, integration 264, plusphpcs,phpstananddeptrac— green on PHP 8.5.10.Assisted by claude-code:claude-opus-5 — Session