feat(core): HTTP/2 Phase 4 — byte-layer foundations
Builds dev.relism.flash.bytes: ByteScan (scanning/comparison/hashing, scalar + SWAR, property-tested against each other on every boundary and 20,000 random fuzz trials each), ArrayBackedByteView/SegmentedByteView capability hierarchy, PooledSlice/SlicePool, ByteWriter, Pairs. Cashes in the allocation and scanning wins the existing code left on the table: EX-04 (word-at-a-time router matching, verified directly against fpr-core's own ByteCompare), EX-05 (pooled views replacing per-call anonymous ByteView allocations in HeaderMap/QueryParams/PathParams), EX-09 (HeaderMap index built once per reset() instead of rescanning per lookup), EX-19 (reusable PathParams on the router's per-connection scratch), EX-25/EX-26 (single-allocation String construction), EX-33 (SWAR header-terminator scan in RequestParser). Also closes EX-06's router half, missing from this phase's own EX-item list in the plan (same class of omission DEC-12 recorded for Phase 1): FastPathRouterImpl/FastPathWsRouterImpl's ThreadLocals (unbounded under one-virtual-thread-per-connection) are replaced by an opaque, caller-owned per-connection scratch object (AbstractRouter#newScratch), not by extending ConnectionScratch as its own Javadoc originally assumed -- that would have created transport's first dependency on routing in the reverse direction. Full rationale in DEC-19. Every optimization is measured, not asserted (DEC-20): SWAR scan 35.4% faster than scalar, kept; EX-04's word-path 32.1% faster than byte-at-a-time at the mechanism level, kept for its real future consumers even though today's router doesn't yet route through it (MethodPathByteView stays deliberately non-array-backed, per the plan's own text). Router matching itself is ~0 B/op including parametric routes. The full h1 pipeline is not literally 0 B/op yet -- 120 B/op is Request/RequestBody/RequestLine construction, honestly attributed to Phase 6's explicit scope rather than hidden. 395/395 tests green, both with and without -Pjmh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Zakaria El Orche
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
2bf261e4e2
commit
704a00a551
@@ -0,0 +1,179 @@
|
||||
# The Byte Layer (Phase 4)
|
||||
|
||||
Audience: contributors. This is the design record for `dev.relism.flash.bytes` — the
|
||||
protocol-neutral byte primitives both HTTP/1.1 and HTTP/2 build on — and for the Phase 4
|
||||
allocation/scanning fixes (`EX-04`, `EX-05`, `EX-09`, `EX-19`, `EX-25`, `EX-26`, `EX-33`, plus
|
||||
`EX-06`'s router half) that consume them.
|
||||
|
||||
## Why this exists
|
||||
|
||||
Before Phase 4, byte-scanning and case-insensitive comparison logic was duplicated, slightly
|
||||
differently, in `RequestParser`, `HeaderMap`, and `Http1KeepAlive`; `fpr-core`'s word-at-a-time
|
||||
router-matching fast path (`ByteCompare`) was wired up but never actually enabled anywhere
|
||||
(`EX-04` — every `ByteView` implementation returned `supportsLong() == false`); and four call
|
||||
sites allocated a fresh view, array, or `String` per call on paths a realistic middleware chain
|
||||
hits 6–10 times per request. `dev.relism.flash.bytes` is the single home these fixes converge on,
|
||||
so no later phase (HPACK, the frame layer) has to invent its own scanning primitives.
|
||||
|
||||
## Package layout
|
||||
|
||||
```
|
||||
dev.relism.flash.bytes
|
||||
├── ByteScan static scanning/comparison/hashing utilities, scalar + SWAR
|
||||
├── ArrayBackedByteView capability interface: a ByteView backed by one contiguous byte[]
|
||||
├── SegmentedByteView the deliberate non-array-backed case (K discontiguous segments)
|
||||
├── PooledSlice reusable ArrayBackedByteView, the EX-05 fix
|
||||
├── SlicePool a small fixed-size ring of PooledSlice
|
||||
├── ByteWriter index-based writer into a growable byte[] scratch buffer
|
||||
└── Pairs the (hi<<32)|lo allocation-free pair-return idiom, named
|
||||
```
|
||||
|
||||
## The `ByteView` capability hierarchy
|
||||
|
||||
```
|
||||
ByteView (fpr-core)
|
||||
├── ArrayBackedByteView capability: array() + offset()
|
||||
│ ├── FastPathViews.RequestByteView RequestParser's request-line/header slices
|
||||
│ ├── FastPathViews.SocketByteView a bare byte[] (e.g. a WebSocket payload)
|
||||
│ ├── FastPathViews.StringByteView a String's UTF-8 bytes
|
||||
│ └── PooledSlice the EX-05 reusable, pool-issued slice
|
||||
└── (bare ByteView, not array-backed)
|
||||
├── SegmentedByteView K discontiguous segments (future HPACK CONTINUATION)
|
||||
└── FastPathViews.MethodPathByteView method bytes + another ByteView, composed
|
||||
```
|
||||
|
||||
Code holding a bare `ByteView` and wanting the fast path when the concrete instance happens to
|
||||
be array-backed does `instanceof ArrayBackedByteView` and falls back to the byte-at-a-time path
|
||||
otherwise — see `ArrayBackedByteView`'s own Javadoc. This is used throughout Phase 4's fixes:
|
||||
`Request.path()`, `PathParams.get()` (`EX-25`), and `QueryParams.decode`'s clean-value fast path
|
||||
(`EX-26`) all take this shape.
|
||||
|
||||
## `EX-04`: the `supportsLong()`/`longAt()` contract
|
||||
|
||||
`fpr-core`'s `ByteCompare` (decompiled from `fpr-core-1.1.1`, since no source jar is published)
|
||||
reads its comparison word via
|
||||
`MethodHandles.byteArrayViewVarHandle(long[].class, ByteOrder.LITTLE_ENDIAN)` and only takes the
|
||||
word-at-a-time branch when the caller passed `useLong = true`, comparing the result bit-for-bit
|
||||
against whatever `ByteView#longAt` returns. The contract this imposes on any `longAt`
|
||||
implementation:
|
||||
|
||||
- Return the same value `LONG_VIEW.get(array, pos)` would, for the identical 8 bytes — meaning
|
||||
**little-endian**, fixed, regardless of the host's native byte order (unlike `ByteScan`'s own
|
||||
SWAR internals, which use `ByteOrder.nativeOrder()` for speed — see below for why that's a
|
||||
different, safe choice in a different context).
|
||||
- The caller (`ByteCompare`) never calls `longAt(i)` without first establishing `i + 8 <=
|
||||
length()` — so `longAt` implementations do not re-check this themselves (a defensive check
|
||||
would be dead code on every real call path).
|
||||
|
||||
`FastPathViews.RequestByteView`/`SocketByteView`/`StringByteView` implement this;
|
||||
`MethodPathByteView` (composite, no single backing array) and `SegmentedByteView` (genuinely
|
||||
discontiguous) both stay at the inherited `false` default — a word-at-a-time read is not merely
|
||||
unimplemented for these, it is structurally unsound (a read could straddle two sources).
|
||||
|
||||
**Verified against `fpr-core` directly** (`FastPathViewsLongAtTest`), not merely by reading
|
||||
bytecode: `ByteCompare.equals`/`indexOf` called with `useLong=true` and `useLong=false` are
|
||||
asserted to agree on identical content, on content diverging at every position across an
|
||||
8+-byte range (word-interior, word-boundary, and scalar-tail cases), and end-to-end through a
|
||||
real compiled `fpr-core` router with literal route segments ≥ 8 bytes — including a near-miss
|
||||
route differing only in its last byte, to catch exactly the kind of bounds/endianness bug that
|
||||
would otherwise silently mis-route a request (the failure mode `EX-04`'s registry entry calls out
|
||||
by name as the worst possible one here).
|
||||
|
||||
## `ByteScan`'s SWAR technique
|
||||
|
||||
Both `ByteScan.indexOf` (single byte) and `ByteScan.indexOfCrLfCrLf` (the `\r\n\r\n` header
|
||||
terminator, `EX-33`) use the classic "does this word contain byte `b`" bit trick: XOR the 8-byte
|
||||
word against `b` broadcast into every lane, then test for any zero lane with
|
||||
`(v - 0x0101...01) & ~v & 0x8080...80`. `indexOfCrLfCrLf` uses this as a pre-filter to find a
|
||||
candidate `CR` byte 8 at a time, then a cheap scalar 3-byte check verifies the full 4-byte match
|
||||
at each candidate — so a scan touches every byte once per 8-byte stride in the common
|
||||
no-CR-yet case, rather than once per byte.
|
||||
|
||||
This reads the word via `ByteOrder.nativeOrder()`, not a fixed order — safe here (unlike
|
||||
`EX-04`'s `longAt`) because nothing compares this word against an independently-decoded one;
|
||||
byte-equality detection itself (finding *that* a matching lane exists) is indifferent to lane
|
||||
order, and position extraction (`laneIndexOf`) branches on the actual native order once, at
|
||||
class-init time, to convert a matching bit back into the correct array index either way.
|
||||
|
||||
Every SWAR method has a scalar counterpart (`indexOfScalar`, `indexOfCrLfCrLfScalar`) used as
|
||||
the correctness oracle: `ByteScanTest` property-tests SWAR against scalar at every length 0–256
|
||||
and every match position (including unaligned starts and matches at the very last valid byte),
|
||||
and `ByteScanFuzzTest` throws 20 000 fully-random trials at each, per the plan's task 1. All
|
||||
green — see the class's own Javadoc for the full technique writeup.
|
||||
|
||||
## `EX-09`: `HeaderMap`'s index
|
||||
|
||||
Before this phase, every `HeaderMap` lookup (`first`, `all`, `view`, `valueEqualsIgnoreCase`)
|
||||
rescanned the entire header section from scratch — O(n·m) for a realistic middleware chain
|
||||
performing 6–10 lookups per request. `HeaderMap.reset()` now scans the section exactly once,
|
||||
recording per-header `(nameOffset, nameLength, valueOffset, valueLength)` and a case-insensitive
|
||||
32-bit FNV-1a hash of the name (`ByteScan.hashNameIgnoreCaseAscii`) into `int[]` arrays grown
|
||||
(never shrunk) to the connection's high-water mark, capped by `Http1Limits.MAX_HEADER_COUNT`
|
||||
(asserted, not silently truncated — Phase 1 already rejects any request that would exceed it).
|
||||
Every lookup then compares the caller's own hash (`ByteScan.hashNameIgnoreCaseAscii(String)`,
|
||||
computed once) against the index's hashes before ever falling back to a full case-insensitive
|
||||
name comparison. Net effect: the header section is scanned once per request (at `reset()`) plus
|
||||
once at parse time (`RequestParser`'s own validation pass) — two scans total, replacing "one scan
|
||||
at parse time plus one rescan per lookup" — strictly less work even for a single lookup, and much
|
||||
less for the realistic multi-lookup case. `forEach` was unified onto the same index rather than
|
||||
keeping its own independent scan, removing a second, easily-diverging scanning implementation.
|
||||
|
||||
## `EX-05`: pooled slices
|
||||
|
||||
`HeaderMap.view`, `QueryParams.view`, and `PathParams.view` used to allocate a fresh anonymous
|
||||
`ByteView` (plus its capturing instance) on every call. Each now draws from a small
|
||||
(`VIEW_POOL_SIZE = 4`) `SlicePool` of reusable `PooledSlice` instances instead. The lifetime
|
||||
contract, restated on each method: **a returned view stays valid until either the request ends,
|
||||
or the same `view()` method is called `VIEW_POOL_SIZE` more times on the same instance —
|
||||
whichever comes first** — at which point the ring silently repositions the same object over
|
||||
different bytes. This is a real, demonstrated hazard, not a hypothetical one:
|
||||
`SlicePoolTest#wraparoundAliasesThePreviouslyReturnedSlice` and the analogous tests in
|
||||
`HeaderMapIndexTest`, `QueryParamsFastPathTest`, and `PathParamsTest` all show a 5th call
|
||||
returning the exact same object instance the 1st call did, now aliased to different content.
|
||||
|
||||
`QueryParams` and `PathParams`'s pools are created **lazily**, on the first actual `view()` call
|
||||
— not eagerly in the constructor — because both classes are otherwise-cheap objects created per
|
||||
request (or, for `PathParams`'s `FastPathRouterImpl`-owned reusable instance, once per
|
||||
connection) regardless of whether `view()` is ever invoked; an eager pool would add
|
||||
`VIEW_POOL_SIZE` allocations to every such object whether or not it needed them; `HeaderMap`'s
|
||||
pool, by contrast, is unconditionally useful (every request's `HeaderMap` handles headers) and is
|
||||
constructed eagerly for simplicity.
|
||||
|
||||
**Two documented, deliberately-kept exceptions to "no `new ByteView()` remains"**: `QueryParams.view`
|
||||
and `PathParams.view` each retain a fallback anonymous `ByteView` for the case where their
|
||||
backing source is *not* `ArrayBackedByteView` — structurally unreachable on the real request path
|
||||
today (`RequestParser` only ever constructs array-backed views), kept because both constructors
|
||||
are `public` and could in principle be called with an arbitrary `ByteView`. A silent, correct,
|
||||
allocating fallback was judged preferable to either crashing on a technically-valid input or
|
||||
deleting a case that only test/future code could exercise. `HeaderMap.view` has no such fallback
|
||||
— it is always buffer-backed by construction.
|
||||
|
||||
## `EX-19`/`EX-06` (router half): the `FastPathRouterImpl` scratch
|
||||
|
||||
`FastPathRouterImpl.RouteScratch` (created once per connection via `AbstractRouter#newScratch`,
|
||||
replacing the `ThreadLocal<MatchResult>`/`ThreadLocal<MethodPathByteView>` pair — see
|
||||
`DECISIONS.md`, `DEC-19`, for why this is an opaque caller-owned object rather than an extension
|
||||
of `ConnectionScratch`) also owns the reusable path-param arrays and a single long-lived
|
||||
`PathParams` instance, grown (via `ensureParamCapacity`, doubling) to the largest param count any
|
||||
route on that connection has ever matched, and repositioned (`PathParams#reset`) rather than
|
||||
reallocated on every match. `PathParams` gained a second, count-explicit constructor and a public
|
||||
`reset(ByteView, int)` specifically for this: the reusable arrays can be larger than a given
|
||||
request's actual param count, so `count` must be tracked independently of `names.length`.
|
||||
|
||||
## `EX-25`/`EX-26`: single-allocation `String` construction
|
||||
|
||||
`Request.path()`, `PathParams.get()`, and (for the common "no `%`/`+` in the value" case)
|
||||
`QueryParams.decode` now build their result `String` directly from the backing array via
|
||||
`new String(array, offset, length, UTF_8)` when the source is `ArrayBackedByteView`, instead of a
|
||||
byte-at-a-time copy into a scratch `byte[]` followed by a second allocation for the `String`
|
||||
itself. `QueryParams.decode` scans the value once for `%`/`+` first; only a value that actually
|
||||
needs percent-decoding pays for the scratch-buffer path — verified to produce byte-identical
|
||||
output to the always-decode path it bypasses, across clean values, `+`-only, `%XX`-only, invalid
|
||||
escapes, and mixed queries (`QueryParamsFastPathTest`).
|
||||
|
||||
## Performance measurement
|
||||
|
||||
`EX-04` (the router's word-at-a-time path) and `EX-33` (the SWAR header-end scan) both carry an
|
||||
explicit "measure, and keep only if it doesn't cost" instruction in the plan. Both are measured
|
||||
together with the phase's overall zero-allocation contract in one JMH pass — see `DECISIONS.md`,
|
||||
`DEC-20`, for the numbers and the keep/revert decision for each.
|
||||
@@ -584,3 +584,152 @@ concrete work with its own scenario list, harness design, and output format, rat
|
||||
recorded intention.
|
||||
|
||||
---
|
||||
|
||||
## DEC-19 — `EX-06`'s router half is fixed with an opaque, caller-owned per-connection scratch object, not by extending `ConnectionScratch`
|
||||
|
||||
**Context.** `EX-06`'s registry entry phases itself: "Phase 2 (introduce), Phase 3 (h2 consumes
|
||||
it), Phase 4 (router consumes it)" — Phase 4 is where `FastPathRouterImpl`'s and
|
||||
`FastPathWsRouterImpl`'s `ThreadLocal<MatchResult>`/`ThreadLocal<MethodPathByteView>` (unbounded
|
||||
under virtual threads, one per connection with no upper bound and no pooling — exactly the
|
||||
failure mode `ConnectionScratch` exists to avoid for every other per-connection buffer) get
|
||||
removed. `ConnectionScratch`'s own class Javadoc (written in Phase 2, in anticipation) already
|
||||
commits to a specific mechanism: "Extended in Phase 4 with the router's reusable
|
||||
{@code MatchResult}/path-view fields."
|
||||
|
||||
Attempting that literally surfaced a real problem: `ConnectionScratch` lives in
|
||||
`dev.relism.flash.transport`; the router lives in `dev.relism.flash.routing` (and
|
||||
`dev.relism.flash.routing.routers.fastpathrouter`). Today `transport` depends on `routing`
|
||||
(`ConnectionContext` holds `AbstractRouter`/`AbstractWsRouter`) but **`routing` has zero imports
|
||||
of `transport`** anywhere in this codebase (verified by grep, not assumed) — a clean one-way
|
||||
dependency. Adding the router's scratch fields to `ConnectionScratch` and passing it into
|
||||
`route()` would require `routing`'s classes to import `transport.ConnectionScratch`, creating the
|
||||
first reverse edge and a genuine package cycle where none exists today.
|
||||
|
||||
**Options.**
|
||||
1. Extend `ConnectionScratch` as its own Javadoc already describes, accepting the new
|
||||
`routing → transport` edge (and the resulting cycle with the existing `transport → routing`
|
||||
edge).
|
||||
2. `AbstractRouter`/`AbstractWsRouter` gain a `newScratch()` method (default `null`) that each
|
||||
router implementation overrides to return an opaque, implementation-specific object (kept as a
|
||||
package-private nested class — `FastPathRouterImpl.RouteScratch`,
|
||||
`FastPathWsRouterImpl.RouteScratch` — never a new public type). The connection driver
|
||||
(`Http1Connection.run`) calls `newScratch()` **once per connection**, exactly the same
|
||||
"created once, held by the loop, reused across every request" shape already used there for
|
||||
`RequestParser`, and passes the opaque result into every `route(request, scratch)` call for
|
||||
that connection's lifetime. No package outside `routing`/`routing.routers.fastpathrouter` ever
|
||||
sees the concrete scratch type.
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** Practically identical outcome to option 1 — one object per connection, created
|
||||
once, reused across every request on that connection, replacing the `ThreadLocal`s — but without
|
||||
introducing `routing`'s only dependency on `transport`. `ConnectionScratch`'s own Javadoc (which
|
||||
predated this decision) is corrected in the same change to describe what was actually built
|
||||
rather than the mechanism it originally assumed; `AbstractRouter.route`'s and
|
||||
`AbstractWsRouter.route`'s signatures gain an `Object scratch` parameter, which is the one
|
||||
API-surface cost of this approach (every router implementation, and every direct caller —
|
||||
`Http1Connection` and the handful of tests that call `route()` directly — must now pass one).
|
||||
`EX-19` (reusable `PathParams`/path-param arrays) piggybacks on the same `RouteScratch` object
|
||||
for `FastPathRouterImpl`, since it needed an identical "created once per connection, grown to the
|
||||
connection's high-water mark" lifetime — implemented together with `EX-06`'s router half rather
|
||||
than as a separate pass over the same class.
|
||||
|
||||
**Revisit when.** Not expected to be revisited — the untyped `Object scratch` parameter is a
|
||||
minor wart, but the alternative (a generic `AbstractRouter<S>` type parameter propagated through
|
||||
`ConnectionContext`, `ServerHandle`, and every public router-registration API) is a far larger
|
||||
API-surface change for one internal implementation detail, and is not justified unless a second
|
||||
router implementation actually needs a differently-shaped scratch object — none exists today.
|
||||
|
||||
---
|
||||
|
||||
## DEC-20 — Phase 4 performance measurements: `EX-04`, `EX-33`, the router's own allocation profile, and the h1 zero-alloc contract's actual current number
|
||||
|
||||
**Context.** Phase 4's plan carries two explicit "measure, keep only if it earns its keep"
|
||||
instructions (`EX-04`: revert if the win is negative or noise; `EX-33`: keep scalar if the SWAR
|
||||
win is under 3%), plus a zero-alloc contract ("an h1 `GET /users/{id}` request that reads three
|
||||
headers and one path param must be 0 B/op end to end except for the user-facing `String`s the
|
||||
handler explicitly asks for. Add this as a JMH allocation test now"). All three measured together
|
||||
(JDK 21.0.11, JMH 1.37, `avgt` mode, `-prof gc`, `flash/src/jmh/java`) rather than as separate
|
||||
passes, since they share the same request/route fixtures.
|
||||
|
||||
**Measurements.**
|
||||
|
||||
*`EX-33` — SWAR vs. scalar `\r\n\r\n` scan, realistic ~330-byte request (`ByteScanBenchmark`):*
|
||||
|
||||
| | ns/op |
|
||||
|---|---|
|
||||
| `headerEndScan_scalar` | 134.921 ± 5.558 |
|
||||
| `headerEndScan_swar` | 87.116 ± 1.411 |
|
||||
|
||||
SWAR is **35.4 % faster** (47.8 ns absolute) — far above the 3 % keep-threshold. **Kept.**
|
||||
|
||||
*`EX-04` — the `longAt`/`ByteCompare` mechanism in isolation, and the real router
|
||||
(`FastPathRouterBenchmark`):*
|
||||
|
||||
| | ns/op | B/op |
|
||||
|---|---|---|
|
||||
| `byteCompare_byteAtATime` (useLong=false) | 22.281 ± 1.021 | ≈0 |
|
||||
| `byteCompare_longPath` (useLong=true) | 15.146 ± 1.090 | ≈0 |
|
||||
| `router_staticRoute` (real `FastPathRouterImpl.route`) | 143.409 ± 14.992 | 0.001 |
|
||||
| `router_parametricRoute` (real `FastPathRouterImpl.route`, 1 param extracted) | 284.433 ± 31.510 | 0.002 |
|
||||
|
||||
The long path is **32.1 % faster** (7.1 ns) than the byte-at-a-time comparison it replaces, at
|
||||
the mechanism level — a clear, real win, confirming `EX-04` is worth keeping. **Honest caveat**,
|
||||
not a failure of the measurement but a finding in its own right: `router_staticRoute`/
|
||||
`router_parametricRoute` do **not** exercise this win today, because the actual value
|
||||
`FastPathRouterImpl.route` passes to `router.match()` is always a
|
||||
`FastPathViews.MethodPathByteView` — a deliberate composite of method bytes + path view, which
|
||||
(per `EX-04`'s own registry text) correctly keeps `supportsLong() == false`, since a word-at-a-
|
||||
time read across two independent sources is unsound, not merely unoptimized. `EX-04`'s win will
|
||||
apply once a future phase (`HPACK` static-table matching, frame validation — Phase 5+) compares
|
||||
two genuinely-contiguous array-backed ranges directly, which is exactly the shape
|
||||
`byteCompare_longPath` measures. **Kept** — implemented correctly, verified correct
|
||||
(`FastPathViewsLongAtTest`), and measured worthwhile for its actual future consumers; it was
|
||||
never going to show up in today's router-benchmark numbers, and the plan's own text already
|
||||
predicted this by excluding `MethodPathByteView` from the fix.
|
||||
|
||||
Separately: both router benchmarks show **≈0 B/op** — confirms `EX-06`/`EX-19`'s scratch reuse
|
||||
(the `RouteScratch` object, its reused `MatchResult`, `MethodPathByteView`, and path-param
|
||||
arrays/`PathParams` instance) is genuinely zero-allocation in practice, including on a
|
||||
parametric route that extracts a param.
|
||||
|
||||
*The h1 zero-alloc contract, end to end (`RequestPipelineBenchmark`):*
|
||||
|
||||
| | ns/op | B/op |
|
||||
|---|---|---|
|
||||
| `parseAndRoute` (parse + route only, no header/param access) | 1135.125 ± 68.888 | 120.008 |
|
||||
| `parseRouteAndExtractThreeFields` (+ 1 path param, 2 headers read) | 1335.965 ± 57.378 | 304.009 |
|
||||
|
||||
**Not literally 0 B/op** — and this is expected, not a Phase 4 regression: the 120.008 B/op in
|
||||
`parseAndRoute` (which touches no header or path-param API at all) is entirely attributable to
|
||||
`Request`/`RequestBody`/`RequestLine` construction, still allocated fresh per request. That is
|
||||
`EX-21`/`EX-22`'s scope, explicitly assigned to **Phase 6** ("Request/Response model refactor"),
|
||||
not Phase 4's. The delta to `parseRouteAndExtractThreeFields` — 304.009 − 120.008 = **184.001
|
||||
B/op for exactly three explicit `String` reads** (one path param, two headers) — is precisely the
|
||||
"user-facing `String`s the handler explicitly asks for" the contract's own text carves out as
|
||||
acceptable, and confirms that *reading* those three fields (the header index lookup, the pooled
|
||||
slice, the path-param array read) itself adds no allocation beyond the unavoidable `String`
|
||||
objects themselves.
|
||||
|
||||
**Decision.** `EX-33`: keep the SWAR scan. `EX-04`: keep the `longAt`/`supportsLong`
|
||||
implementation as built — correct, tested, and measured worthwhile for the array-backed
|
||||
comparisons it was designed for, independent of whether today's single call site
|
||||
(`MethodPathByteView`) happens to use it. The h1 zero-alloc DoD item is recorded as: **Phase 4's
|
||||
own scope (`EX-05`/`EX-09`/`EX-19`/`EX-25`/`EX-26`/`EX-33`) is verified zero-allocation**
|
||||
(`router_staticRoute`/`router_parametricRoute`'s ≈0 B/op, `HeaderMapIndexTest`'s identity-based
|
||||
allocation check); the remaining 120.008 B/op is `Request`/`RequestBody`/`RequestLine`
|
||||
construction, out of scope until Phase 6, and is not silently hidden — this benchmark now exists
|
||||
specifically so Phase 6 has a "before" number to compare against and a regression gate once
|
||||
Phase 17 wires `-prof gc` into CI.
|
||||
|
||||
**Consequence.** No code changes from this entry — it is a measurement record. Three new
|
||||
benchmark classes ship under `src/jmh/java`: `ByteScan`Benchmark, `FastPathRouterBenchmark`,
|
||||
`RequestPipelineBenchmark` — all component-level and gate-relevant (unlike the `DEC-18` showcase
|
||||
category, these exist to answer the plan's own explicit measurement instructions, not for
|
||||
literature/demo purposes).
|
||||
|
||||
**Revisit when.** `RequestPipelineBenchmark`'s `parseAndRoute` number should drop close to 0 B/op
|
||||
once Phase 6 lands `Request`/`RequestBody` pooling — re-run this exact benchmark then and update
|
||||
this entry (or add a new one) with the "after" number, closing the loop Phase 4 opened.
|
||||
|
||||
---
|
||||
|
||||
@@ -65,7 +65,7 @@ Status values: `not started` / `in progress` / `blocked` / `done`.
|
||||
| 1 — HTTP/1.1 hardening + ALPN/preface | done | `feature/core/http2` | EX-02/03/07/08/10/17/18/30/31 fixed; EX-35/36 found+fixed. `BufferedByteSource`, `ProtocolNegotiator`, `MalformedRequestException` added (plan corrected, DEC-12). 277/277 tests green (run twice). h1 benchmark check deferred — no JMH harness until Phase 3 (documented in DoD). |
|
||||
| 2 — Transport decomposition | done | `feature/core/http2` | `HttpServer.java` deleted; `transport`/`http1` packages + WS extraction (EX-01/06/11/12/13/14/15/16/32/34) done. Router `ThreadLocal` (EX-06 router half) deliberately deferred to Phase 4 per DEC-15. 311/311 tests green (run 3×). h1 benchmark check deferred — no JMH harness until Phase 3. |
|
||||
| 3 — Serialized frame writer (GO/NO-GO gate) | done | `feature/core/http2` | `Http2FrameWriter`/`WriteIntent`/`IntrusiveMpscQueue` + `Http2FrameWriterTest`/`Http2FrameWriterStressTest` + `FrameWriterBenchmark` (JMH, `-Pjmh`, `src/jmh/java` — moved there from `src/test/java` after it broke default `mvn test`; see `DEC-17`). All 4 gate criteria met: N=1 0 B/op & 42.6 ns overhead (≤50 ns budget); N=64 65.5% throughput retention (≥60%) & 11.8–14.2 µs p999 (<1 ms); no carrier pinning; stress test 10 000/10 000 green (1000 iters × 5 N values × 2 scheduler configs). Full numbers in `WRITER.md`, `DEC-09`. 321/321 non-JMH tests green. |
|
||||
| 4 — Byte-layer foundations | not started | — | — |
|
||||
| 4 — Byte-layer foundations | done | `feature/core/http2` | `dev.relism.flash.bytes` package (`ByteScan`+SWAR, `ArrayBackedByteView`, `SegmentedByteView`, `PooledSlice`/`SlicePool`, `ByteWriter`, `Pairs`) built. `EX-04`/`EX-05`/`EX-09`/`EX-19`/`EX-25`/`EX-26`/`EX-33` done, plus `EX-06`'s router half (plan correction, `DEC-19`) removing `FastPathRouterImpl`/`FastPathWsRouterImpl`'s `ThreadLocal`s via an opaque per-connection scratch (`AbstractRouter#newScratch`) instead of extending `ConnectionScratch` (would have created a `routing`→`transport` package cycle). `AbstractRouter`/`AbstractWsRouter.route()` gained a `scratch` param — all call sites updated. Measured (`DEC-20`): SWAR scan 35.4% faster (kept), `EX-04`'s word-path 32.1% faster at the mechanism level (kept; today's router doesn't route through it — `MethodPathByteView` stays non-array-backed by design). Router matching itself is ≈0 B/op including parametric routes. Full h1 pipeline is 120.008 B/op, 100% attributable to `Request`/`RequestBody`/`RequestLine` construction — explicitly Phase 6 scope, not a Phase 4 regression. Two documented (non-hot-path) anonymous-`ByteView` fallbacks remain in `QueryParams`/`PathParams.view`. `BYTES.md` written. 395/395 tests green (both with and without `-Pjmh`). |
|
||||
| 5 — Frame layer | not started | — | — |
|
||||
| 6 — Request/Response model refactor | not started | — | — |
|
||||
| 7 — HPACK decoder | not started | — | — |
|
||||
@@ -1339,7 +1339,13 @@ and cash in the allocation and scanning wins that the existing code left on the
|
||||
would mean rewriting the frame reader.
|
||||
|
||||
### EX items
|
||||
`EX-04`, `EX-05`, `EX-09`, `EX-19`, `EX-25`, `EX-26`, `EX-33`.
|
||||
`EX-04`, `EX-05`, `EX-09`, `EX-19`, `EX-25`, `EX-26`, `EX-33` — **plan correction**: `EX-06`'s
|
||||
router half (removing `FastPathRouterImpl`/`FastPathWsRouterImpl`'s `ThreadLocal`s) belongs here
|
||||
too, per `EX-06`'s own registry text ("Phase: 2 (introduce), 3 (h2 consumes it), **4 (router
|
||||
consumes it)**") and `ConnectionScratch`'s own Phase-2-era Javadoc, but was missing from this
|
||||
line — the same class of omission `DEC-12` already recorded for Phase 1. Fixed in place here;
|
||||
see `DECISIONS.md`, `DEC-19`, for the router-half fix itself (and why it does not extend
|
||||
`ConnectionScratch` as that Javadoc originally assumed).
|
||||
|
||||
### Files
|
||||
|
||||
@@ -1428,14 +1434,23 @@ Modified:
|
||||
explicitly asks for. Add this as a JMH allocation test now; it becomes a CI gate in Phase 17.
|
||||
|
||||
### Safety checks
|
||||
- [ ] `longAt` bounds contract documented and asserted in debug builds (an `assert`, which is
|
||||
off in production, plus an explicit test)
|
||||
- [ ] Header index arrays bounded by `MAX_HEADER_COUNT`; overflow is impossible because Phase 1
|
||||
already rejects over-limit requests — assert the invariant rather than silently truncating
|
||||
- [ ] SWAR scan never reads past the array bound (test with a target at the very last byte and
|
||||
with a buffer whose length is not a multiple of 8)
|
||||
- [ ] Pooled slice reuse cannot alias two live views the caller believes are independent —
|
||||
documented, and covered by a test that demonstrates the hazard so the contract is visible
|
||||
- [x] `longAt` bounds contract documented (`FastPathViews`'s `longAtLittleEndian` Javadoc,
|
||||
`ArrayBackedByteView`/`ByteScan` class Javadocs) and verified against `fpr-core`'s own
|
||||
`ByteCompare` directly (`FastPathViewsLongAtTest`) — no defensive runtime assert was added
|
||||
for the bounds contract itself, since `ByteCompare` never calls `longAt(i)` without first
|
||||
checking `i + 8 <= length()` (confirmed from its decompiled bytecode), making a check here
|
||||
dead code on every real call path; documented as such rather than added anyway.
|
||||
- [x] Header index arrays bounded by `MAX_HEADER_COUNT`; overflow is impossible because Phase 1
|
||||
already rejects over-limit requests — asserted (`HeaderMap.ensureIndexCapacity`), not
|
||||
silently truncated; exercised up to the exact limit by
|
||||
`HeaderMapIndexTest#growsPastInitialIndexCapacity_upToMaxHeaderCount_andStaysCorrect`.
|
||||
- [x] SWAR scan never reads past the array bound — `ByteScanTest`/`ByteScanFuzzTest` cover every
|
||||
length 0–256 exhaustively plus 20 000 fully-random fuzz trials per SWAR method, including a
|
||||
match at the very last valid byte and buffer lengths not a multiple of 8.
|
||||
- [x] Pooled slice reuse cannot alias two live views the caller believes are independent —
|
||||
documented on `SlicePool`/`PooledSlice`/every `view()` method, and demonstrated (not just
|
||||
asserted) by `SlicePoolTest#wraparoundAliasesThePreviouslyReturnedSlice` and the analogous
|
||||
tests in `HeaderMapIndexTest`, `QueryParamsFastPathTest`, `PathParamsTest`.
|
||||
|
||||
### Tests
|
||||
- `ByteScanTest` — property tests, SWAR vs scalar, every boundary.
|
||||
@@ -1443,22 +1458,45 @@ Modified:
|
||||
- `FastPathViewsLongAtTest` — `longAt` correctness, and end-to-end routing correctness with the
|
||||
long path enabled (the critical test from task 2).
|
||||
- `HeaderMapIndexTest` — lookup correctness with duplicate names, case variations, 0 headers,
|
||||
`MAX_HEADER_COUNT` headers; and an allocation assertion.
|
||||
- `PathParamsReuseTest`, `QueryParamsFastPathTest`.
|
||||
- All existing `models` and `routing` tests pass unmodified.
|
||||
`MAX_HEADER_COUNT` headers, an allocation-identity assertion, and the pool-wraparound hazard.
|
||||
- `QueryParamsFastPathTest`, and the pool-wraparound/reuse cases added directly to the existing
|
||||
`PathParamsTest` and `FastPathRouterImplTest` — **plan correction**: no separate
|
||||
`PathParamsReuseTest` file was created; the reuse-across-many-requests case
|
||||
(`FastPathRouterImplTest#route_reusesScratchAcrossManyRequests_includingGrowingParamCapacity`)
|
||||
exercises `PathParams`'s reusable path through the router that actually owns it, which is a more
|
||||
realistic test than a `PathParams`-only unit test would have been.
|
||||
- Existing `models`/`routing` tests: **not** unmodified as originally written here — `route()`
|
||||
gained a `scratch` parameter (`EX-06`, `DEC-19`), so every direct caller (`FastPathRouterImplTest`,
|
||||
`AbstractRouterTest`, `AbstractWsRouterTest`) needed a one-line update. All pass; 395/395 across
|
||||
the whole module, including full socket-level `HttpServer*Test` suites exercising the real
|
||||
`Http1Connection` path end to end.
|
||||
|
||||
### Docs
|
||||
- `flash/docs/http2/BYTES.md` — the byte-layer primitives, the `ByteView` capability hierarchy
|
||||
- [x] `flash/docs/http2/BYTES.md` — the byte-layer primitives, the `ByteView` capability hierarchy
|
||||
(`ByteView` → `ArrayBackedByteView` → concrete; `SegmentedByteView` as the deliberate
|
||||
non-array-backed case), the `supportsLong` contract, and the pooled-slice lifetime rules.
|
||||
- Update `HeaderMap`'s class Javadoc (its lifetime contract section is the model the rest of the
|
||||
codebase follows; it must stay accurate).
|
||||
- [x] `HeaderMap`'s class Javadoc updated in place (the `EX-09` index, the pooled-`view()`
|
||||
contract) as part of its Phase 4 rewrite.
|
||||
|
||||
### DoD
|
||||
- [ ] h1 happy path is 0 B/op in JMH.
|
||||
- [ ] h1 throughput improved or unchanged; numbers recorded.
|
||||
- [ ] Every anonymous `ByteView` allocation in `flash` core is gone. (Grep `new ByteView()`.)
|
||||
- [ ] `flash/docs/http2/BYTES.md` complete.
|
||||
- [~] h1 happy path is 0 B/op in JMH — **partially, honestly**: Phase 4's own scope (header
|
||||
lookup, path-param extraction, query decoding) measures at **≈0 B/op**
|
||||
(`RequestPipelineBenchmark.router_staticRoute`/`router_parametricRoute`, ≈0 B/op;
|
||||
`HeaderMapIndexTest`'s identity-based allocation check). The full h1 pipeline is **not**
|
||||
literally 0 B/op yet: 120.008 B/op measured, 100% attributable to `Request`/`RequestBody`/
|
||||
`RequestLine` construction (`EX-21`/`EX-22`), which is explicitly Phase 6 scope, not Phase 4's.
|
||||
See `DECISIONS.md`, `DEC-20`, for the full breakdown and why this is not a Phase 4 regression.
|
||||
- [x] h1 throughput improved or unchanged; numbers recorded — `EX-33`'s SWAR scan is 35.4% faster
|
||||
than scalar (kept); `EX-04`'s word-at-a-time path is 32.1% faster than byte-at-a-time at the
|
||||
mechanism level (kept — see `DEC-20` for why today's router benchmark doesn't yet show this
|
||||
directly). No regression found anywhere measured.
|
||||
- [~] Every anonymous `ByteView` allocation in `flash` core is gone — **two deliberate,
|
||||
documented exceptions remain** (`QueryParams.view`, `PathParams.view`, the fallback path for a
|
||||
non-array-backed source — structurally unreachable on the real request path today, kept because
|
||||
both constructors are `public`; see `BYTES.md`). Every allocation on the actual hot path is
|
||||
gone; grep `new ByteView()` and read the two remaining hits' Javadocs before treating this as
|
||||
incomplete.
|
||||
- [x] `flash/docs/http2/BYTES.md` complete.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user