Revamp benchmark suite with storefront workloads and guarded comparisons - #70
Merged
Conversation
This reverts commit 1a7ae91.
- ThemeBench read template sources inside the timed loop, so benchTokenize and benchParse measured the filesystem: 313us of the ~840us per rev were file_get_contents calls. Sources are now read once in setUp. - The theme used a pluralize filter that does not exist in this library. Unknown filters return their input unmodified, so the cart line rendered as "Cart: 3 3". Added fixture_pluralize and asserted on the rendered output so a missing filter cannot pass silently again. - The CI segfault exclusion only named benchLoadAndRenderVarExporter, while the new benchBuildVarExporter subject ran unfiltered. - Restored warmup, dropped in the suite revamp. Passing it on the command line rather than via attributes keeps the base and PR runs symmetric regardless of what the base commit declares. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LiquidBench was the only consumer of the old Shopify theme fixture, so deleting it left the whole harness unreachable: ThemeRunner, the theme test template pair, CommentFormTag, PaginateTag, CustomFilters, Database with its vision.database.yml, and the 34 template files under performance/tests. symfony/yaml went with Database, its only user, and the phpstan baseline entries for the removed files are gone too. DatabaseDrop stays: OperationBench still uses it for the drop benchmarks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renames the theme to `storefront` (it renders four page types, not one
collection), replaces the fixture's inline data with a typed drop layer, and
deepens the theme into something a real storefront page would look like.
Data layer. `Database` is a typed-drop factory rather than the generic
array-wrapping table store the deleted Shopify harness used: ten drops, one per
entity, no recursive conversion and no phpstan suppressions. Two rules keep the
benchmark measuring the library instead of the fixture — it assigns rather than
computes, and it memoizes nothing, so #[Cache] starts cold on every render and no
state is shared across revolutions. Building the fixture is ~6% of benchRender.
Drop resolution strategies are assigned deliberately, each where a real drop
would use it: public typed properties for stored fields, methods for derived
values, #[Cache] on the one method that walks every variant and is read by three
snippets, and liquidMethodMissing only on metafields, where keys are genuinely
arbitrary. DatabaseDrop is gone and performance/Shopify/ with it.
Theme. Twenty-nine templates across the same four pages: real head metadata,
nested navigation, breadcrumbs, a filter sidebar, a variant picker, a specs
table, a multi-column footer. It now uses the tags a real theme uses — case,
capture, cycle, for/else, break, continue and {% liquid %} — and the fabricated
padding that repeated inventory_label five times per product is gone. Tags real
themes barely touch stay out; they belong in micro subjects.
Verification. Benchmarks run with library defaults, but the fixture test renders
every page with strictVariables, strictFilters and rethrowErrors on, so a missing
variable, filter or swallowed error fails the suite rather than rendering as
empty output. No snapshots: the theme is expected to keep growing, and a
regenerated snapshot asserts nothing.
performance/README.md records the design constraints that are invisible in the
code, and the deferred list — per-tag micro subjects, the drop miss path,
size-parameterized scaling, coverage-only tags, TemplateCacheBench's shape, and
the CI base run's missing --group=macro.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Convention, from the two liquidMethodMissing implementations in src/Drops: MetafieldsDrop declares #[DropDynamicProperties], so toArray() can see the dynamic keys. It does not affect Drop::__get resolution. The eleven-key product row shape was written out twice; it is now one @phpstan-type, as src/Parse and src/Nodes already do for their shapes. page_title restated titles that Database entities already owned, in a match inside StorefrontTheme — which also made that class change for two reasons. Titles shared by an entity and by the page title now live in one constant each, and the product title is read from the row rather than from a built drop: resolving the duplication through Database::product() would have rebuilt all 24 products per render, undoing the construction fix. Removed ROOT_TEMPLATE_NAME and the default arguments that referenced it — no caller omitted the argument — and the breadcrumb branch for an empty parent, which all three call sites passed a literal for. Corrected three documentation claims that overstated the fixture: the nav is flat by design rather than nested, #[Cache] is read twice per instance on the product page rather than by three snippets, and the ~6% fixture-construction share is a measured reference point with no assertion behind it, so it says how to re-measure. The deferred list gains the liquidMethodMissing hit path, which OperationBench stopped covering when its drop was swapped. Fixture construction re-measured at 6.3% of benchRender. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Scope storefront fixture data and discover templates automatically - Add operation benchmarks and comparator regression tests - Enforce a 5% CI regression threshold
Contributor
PHPBench comparison (PHP 8.3)Benchmark comparison was not generated. |
Contributor
PHPBench comparison (PHP 8.4)Benchmark comparison was not generated. |
Contributor
PHPBench comparison (PHP 8.2)Benchmark comparison was not generated. |
Contributor
PHPBench comparison (PHP 8.5)Benchmark comparison was not generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing