papegaaij opened a new pull request, #1595:
URL: https://github.com/apache/wicket/pull/1595
Draft, for discussion. This collects the WICKET-6774 component state rework
together with a
handful of further allocation and CPU savings found by profiling a real
application, and adds
the benchmark module they were all measured with.
## wicket-benchmarks
A new module, not part of the default reactor build (`-Pbenchmarks`, see its
README).
The component benchmarks used to investigate WICKET-6774 only ever existed
as attachments on
the issue, and they no longer compile: `WicketTester` has moved to its own
module, and since
JDK 23 javac no longer runs annotation processors found on the classpath, so
JMH silently
produces no `BenchmarkList` and the run executes nothing. Keeping them in
the reactor means
they keep compiling.
It now holds three kinds of tool:
* **JMH benchmarks** - `ComponentStateBenchmark`, `PageRenderBenchmark`,
`ResourceNameIteratorBenchmark`, `PageSerializationBenchmark`,
`PageEncryptionBenchmark`.
* **`ComponentFootprint`** - retained heap via JOL and serialized size, per
state shape,
against an identical stateless tree so the difference isolates the state
itself.
* **`WicketContext`** - the shared harness.
## Component state (WICKET-6774)
A component's state is no longer an `Object[]` with a packing convention,
but a
`ComponentState` instance. One final class rather than one subclass per
combination: every
unpacking call site otherwise dispatches over four implementations of the
same six accessors,
which is past the point where HotSpot stops inlining, and a real page
interleaves shapes even
where a benchmark feeding one shape at a time would not show it.
Reading state over an array holding every shape at once, 2 forks, ns/op,
master -> this branch:
| | master | this branch | |
|---|---|---|---|
| `readBehaviorsMixedShapes` | 31.50 | 21.35 | -32% |
| `readMetaDataMixedShapes` | 25.27 | 17.66 | -30% |
| `readModelMixedShapes` | 30.17 | 16.54 | -45% |
| `readBehaviors[AJAX_BEHAVIOR]` | 11.15 | 1.97 | -82%, 72 -> 24 B/op |
| `readBehaviorById` | 2.82 | 1.62 | -43% |
For a component carrying a real `AjaxEventBehavior` the tree is 39.8%
smaller serialized, -72
bytes retained heap and -32 bytes serialized per component - comparable to
the -36.2% reported
on the issue in 2020.
**Behaviour change:** behavior ids are only maintained for stateful
behaviors. Ids can change
for other behaviors, also when combined on the same component. This is
documented in the guide
commit on this branch.
## Resource name iteration
`ResourceStreamLocator` walks a list of candidate filenames for every
property and markup
lookup, once per registered properties loader. Misses are the common case,
because a key is
resolved by climbing the component hierarchy, so every class above the one
that declares it
contributes a full traversal that finds nothing.
`toString()` built each candidate from four `prepend()` calls, and the
locale part went through
`getLocale()` - a `Locale.of()` cache lookup - even though
`LocaleResourceNameIterator.next()`
had just built the identical suffix and thrown it away.
`ResourceUtil#rejectPathSeparators`
separately ran `Locale#toString()` on every call.
`nl_NL` without a style, 3 forks:
| | master | this branch | |
|---|---|---|---|
| `walkAllCandidates` (miss) | 923.8 ns, 1224 B | 139.9 ns, 672 B | -85% /
-45% |
| `firstCandidate` (hit) | 775.2 ns, 600 B | 405.6 ns, 368 B | -48% / -39% |
Both changes were verified by enumeration rather than by argument: every
candidate name with
its locale, style, variation and extension over 6 paths x 2 styles x 2
variations x 7 locales x
5 extension lists x strict/non-strict (9918 names), and 4918 locales for
`rejectPathSeparators`
- byte-identical before and after.
One note for reviewers: `toString()` now takes the locale segment from
`next()` instead of from
`getLocale()`. A subclass supplied through the protected
`newLocaleResourceNameIterator` hook
that overrode `getLocale()` without also overriding `next()` would see a
difference; nothing in
the tree does that.
## Page store
Serialization and encryption run on the request thread after the response
has been flushed, so
this is throughput and thread occupancy rather than user-visible latency.
`JavaSerializer` wrote into a `java.io.ByteArrayOutputStream` with no
initial size, which
starts at 32 bytes and grows by copying everything written so far into a
buffer of twice the
size. Wicket already has a `ByteArrayOutputStream` that chains a new buffer
instead.
`SchemeCrypt` prefixes every ciphertext with a one-byte marker naming the
scheme, and did so by
allocating a new array one byte longer and copying the whole payload in -
and on the way back
copying everything after the marker out again. `AbstractAesGcmCryptScheme`
added a third copy.
`ICryptScheme#encrypt`/`#decrypt` now carry the offsets needed to avoid all
three; the old
signatures remain as default methods. The marker stays `SchemeCrypt`'s
concern, as documented
in the user guide - a scheme still knows nothing about it, only that the
first few bytes of
what it returns are not its own.
| | master | this branch | |
|---|---|---|---|
| serialize, 500 components (37,924 bytes) | 256,714 B/op | 214,763 B/op |
-16% |
| encrypt, 40kB | 18.28 us, 124,496 B | 12.58 us, 44,416 B | -31% / -64% |
| decrypt, 40kB | 14.07 us, 84,408 B | 11.17 us, 44,360 B | -21% / -47% |
Encrypting now allocates 1.11x the payload rather than 3.1x. Serialization
time is unchanged;
what remains there is `ObjectOutputStream`'s handle table, which scales with
the number of
objects in the page graph rather than with its byte count - which is the
thing the component
state rework above reduces.
## How the non-6774 hotspots were found
JFR on a large production application under its Selenium suite, keeping only
samples whose
stack contains `WicketFilter.doFilter`, charging JDK frames to the nearest
non-JDK caller and
bucketing by package. Inside Wicket request handling: page store 23.2% CPU /
12.4% allocation,
render 16.8% / 15.5%, localizer 6.1% / 15.0%.
Two attempts were measured and dropped rather than included: folding the
resource name
iterators into precomputed lists (6x slower on the hit path, because
`getLocale()` costs a
cache lookup per state and the hit path stops early), and a `reset()`-based
restart of the same
iterators (no time win, three new subclass contract hazards).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
