diff --git a/flash/docs/http2/BYTES.md b/flash/docs/http2/BYTES.md new file mode 100644 index 0000000..921d2c3 --- /dev/null +++ b/flash/docs/http2/BYTES.md @@ -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`/`ThreadLocal` 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. diff --git a/flash/docs/http2/DECISIONS.md b/flash/docs/http2/DECISIONS.md index 895821d..dbd4207 100644 --- a/flash/docs/http2/DECISIONS.md +++ b/flash/docs/http2/DECISIONS.md @@ -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`/`ThreadLocal` (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` 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. + +--- diff --git a/flash/docs/http2/IMPLEMENTATION-PLAN.md b/flash/docs/http2/IMPLEMENTATION-PLAN.md index d1a2183..adbc011 100644 --- a/flash/docs/http2/IMPLEMENTATION-PLAN.md +++ b/flash/docs/http2/IMPLEMENTATION-PLAN.md @@ -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. --- diff --git a/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java b/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java new file mode 100644 index 0000000..aef73ef --- /dev/null +++ b/flash/src/jmh/java/dev/relism/flash/RequestPipelineBenchmark.java @@ -0,0 +1,124 @@ +package dev.relism.flash; + +import dev.relism.flash.http.HttpMethod; +import dev.relism.flash.models.Request; +import dev.relism.flash.models.RequestHandler; +import dev.relism.flash.models.SimpleHandler; +import dev.relism.flash.routing.Middleware; +import dev.relism.flash.routing.routers.fastpathrouter.FastPathRouterImpl; +import dev.relism.flash.transport.BufferedByteSource; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.TimeUnit; + +/** + * Phase 4's zero-alloc contract: "an h1 {@code 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 {@code String}s the + * handler explicitly asks for." This benchmark measures the actual current number with + * {@code -prof gc} — see {@code DECISIONS.md}, {@code DEC-20}, for the honest result and why it + * is not literally 0 B/op yet: {@code Request}/{@code RequestBody}/{@code RequestLine} are still + * allocated per request ({@code EX-21}/{@code EX-22}, explicitly Phase 6 scope, not Phase 4's). + * The two benchmark methods below isolate that cost from Phase 4's own scope (header lookups, + * path-param extraction, query decoding) by comparing a route with no header/param access against + * one that performs exactly the access the DoD text describes. + * + *

Uses a hand-rolled repeating {@link InputStream} (never allocates, cycles the same request + * bytes indefinitely) rather than a fresh {@code ByteArrayInputStream}/{@code BufferedByteSource} + * per invocation, so the timed path matches production exactly: one {@link BufferedByteSource} + * created once per connection and reused across every request, per {@code Http1Connection}'s own + * shape — not recreated per benchmark iteration, which would contaminate the measurement with + * harness allocation unrelated to the parser/router/model code under test (the same lesson + * {@code WRITER.md} documents for {@code FrameWriterBenchmark}'s own harness). + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(2) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +public class RequestPipelineBenchmark { + + /** Cycles a fixed byte[] indefinitely — simulates an infinite pipelined keep-alive stream + * of identical requests without allocating anything per read. */ + private static final class RepeatingByteStream extends InputStream { + private final byte[] template; + private int pos; + + RepeatingByteStream(byte[] template) { + this.template = template; + } + + @Override + public int read() { + byte b = template[pos]; + pos = (pos + 1) % template.length; + return b & 0xFF; + } + + @Override + public int read(byte[] dst, int off, int len) { + for (int i = 0; i < len; i++) { + dst[off + i] = template[pos]; + pos = (pos + 1) % template.length; + } + return len; + } + } + + private RequestParser parser; + private BufferedByteSource in; + private FastPathRouterImpl router; + private Object routeScratch; + + @Setup(Level.Trial) + public void setup() { + String req = "GET /users/12345 HTTP/1.1\r\n" + + "Host: api.example.com\r\n" + + "Accept: application/json\r\n" + + "Authorization: Bearer abcdefghijklmnopqrstuvwxyz\r\n" + + "\r\n"; + byte[] template = req.getBytes(StandardCharsets.US_ASCII); + in = new BufferedByteSource(new RepeatingByteStream(template), null); + parser = new RequestParser(64 * 1024); + + router = new FastPathRouterImpl(); + RequestHandler handler = new SimpleHandler((r, res) -> "ok"); + router.doRegister(HttpMethod.GET, "/users/{id}", handler, new Middleware[0]); + router.compile(); + routeScratch = router.newScratch(); + } + + /** Parse + route only — isolates Phase 4's own scope from Request/RequestBody construction + * by not touching header()/param() (the "user-facing String" opt-in the DoD text carves out). */ + @Benchmark + public RequestHandler parseAndRoute() throws IOException { + Request request = parser.parse(in); + request.drain(); + return router.route(request, routeScratch); + } + + /** Parse + route + exactly what the DoD text describes: one path param, two headers read. */ + @Benchmark + public Object parseRouteAndExtractThreeFields() throws IOException { + Request request = parser.parse(in); + RequestHandler handler = router.route(request, routeScratch); + String id = request.param("id"); + String host = request.header("Host"); + String auth = request.header("Authorization"); + request.drain(); + return id.length() + host.length() + auth.length() + (handler != null ? 1 : 0); + } +} diff --git a/flash/src/jmh/java/dev/relism/flash/bytes/ByteScanBenchmark.java b/flash/src/jmh/java/dev/relism/flash/bytes/ByteScanBenchmark.java new file mode 100644 index 0000000..6019fa6 --- /dev/null +++ b/flash/src/jmh/java/dev/relism/flash/bytes/ByteScanBenchmark.java @@ -0,0 +1,65 @@ +package dev.relism.flash.bytes; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.nio.charset.StandardCharsets; +import java.util.concurrent.TimeUnit; + +/** + * {@code EX-33}'s required measurement: "SWAR scan using the same VarHandle long-read technique + * ... Measure — if the win is under 3% on the h1 benchmark, keep the scalar version." Compares + * {@link ByteScan#indexOfCrLfCrLf} (SWAR) against {@link ByteScan#indexOfCrLfCrLfScalar} on a + * realistic HTTP/1.1 request header block. Lives in this package (not {@code src/test/java}) + * specifically to reach the package-private scalar reference method without widening its + * visibility just for a benchmark — see {@code DEC-17} for why JMH sources are kept out of + * {@code src/test/java} generally. + * + *

Run: {@code mvn -Pjmh -pl flash test-compile} then + * {@code java -cp flash/target/test-classes:flash/target/classes:$(mvn -Pjmh -pl flash dependency:build-classpath -Dmdep.outputFile=/dev/stdout -q) + * org.openjdk.jmh.Main ByteScanBenchmark}. Results recorded in {@code DECISIONS.md}, {@code DEC-20}. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(2) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +public class ByteScanBenchmark { + + /** A realistic request: request line + 7 headers + terminator, ~330 bytes. */ + private byte[] requestBuf; + + @Setup(Level.Trial) + public void setup() { + String req = "GET /users/12345?sort=desc&limit=20 HTTP/1.1\r\n" + + "Host: api.example.com\r\n" + + "User-Agent: Mozilla/5.0 (compatible; FlashBench/1.0)\r\n" + + "Accept: application/json\r\n" + + "Accept-Encoding: gzip, deflate, br\r\n" + + "Authorization: Bearer abcdefghijklmnopqrstuvwxyz0123456789\r\n" + + "Cookie: session=xyz123abc; theme=dark; lang=en-US\r\n" + + "Connection: keep-alive\r\n" + + "\r\n"; + requestBuf = req.getBytes(StandardCharsets.US_ASCII); + } + + @Benchmark + public int headerEndScan_swar() { + return ByteScan.indexOfCrLfCrLf(requestBuf, 0, requestBuf.length); + } + + @Benchmark + public int headerEndScan_scalar() { + return ByteScan.indexOfCrLfCrLfScalar(requestBuf, 0, requestBuf.length); + } +} diff --git a/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java b/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java new file mode 100644 index 0000000..5cb6aad --- /dev/null +++ b/flash/src/jmh/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterBenchmark.java @@ -0,0 +1,114 @@ +package dev.relism.flash.routing.routers.fastpathrouter; + +import dev.relism.flash.http.HttpMethod; +import dev.relism.flash.models.Request; +import dev.relism.flash.models.RequestHandler; +import dev.relism.flash.models.SimpleHandler; +import dev.relism.fpr.core.internal.runtime.ByteCompare; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.nio.charset.StandardCharsets; +import java.util.concurrent.TimeUnit; + +/** + * Two related, but distinct, {@code EX-04} measurements — see {@code DECISIONS.md}, {@code DEC-20}, + * for the honest write-up of why they tell different stories. + * + *

{@code router_*}: the plan's literal instruction — "measure the {@code EX-04} win on + * the h1 router benchmark" — exercised through the real, shipped {@link FastPathRouterImpl#route} + * end to end (lazy-compiled route table, {@link FastPathRouterImpl.RouteScratch} reuse, path-param + * extraction included). + * + *

{@code byteCompare_*}: a direct measurement of the mechanism {@code EX-04} actually + * implements ({@code ByteCompare.equals}, {@code useLong} true vs. false) over array-backed + * content shaped like what a future call site (HPACK static-table matching, frame validation) + * would compare. This exists because the router's own match call passes + * {@link FastPathViews.MethodPathByteView} — a deliberate composite, never array-backed (see + * {@code EX-04}'s own registry text: "{@code MethodPathByteView} ... keep[s] the {@code false} + * default") — so {@code router_*} alone cannot show {@code EX-04}'s effect at all; this benchmark + * is what actually answers "is the long path worth what it implements" for future consumers. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(2) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +public class FastPathRouterBenchmark { + + // ── router_*: the real, shipped router, end to end ────────────────────── + + private FastPathRouterImpl router; + private Object scratch; + private Request staticRequest; + private Request paramRequest; + + @Setup(Level.Trial) + public void setupRouter() { + router = new FastPathRouterImpl(); + RequestHandler h = new SimpleHandler((req, res) -> "ok"); + router.doRegister(HttpMethod.GET, "/health", h, new dev.relism.flash.routing.Middleware[0]); + router.doRegister(HttpMethod.GET, "/users/{id}", h, new dev.relism.flash.routing.Middleware[0]); + router.doRegister(HttpMethod.GET, "/users/{id}/posts/{postId}", h, new dev.relism.flash.routing.Middleware[0]); + router.doRegister(HttpMethod.POST, "/users", h, new dev.relism.flash.routing.Middleware[0]); + router.doRegister(HttpMethod.GET, "/api/v1/products/{category}/{id}", h, new dev.relism.flash.routing.Middleware[0]); + router.compile(); + scratch = router.newScratch(); + + staticRequest = mockRequest(HttpMethod.GET, "/health"); + paramRequest = mockRequest(HttpMethod.GET, "/users/12345/posts/67890"); + } + + @Benchmark + public RequestHandler router_staticRoute() { + return router.route(staticRequest, scratch); + } + + @Benchmark + public RequestHandler router_parametricRoute() { + return router.route(paramRequest, scratch); + } + + private static Request mockRequest(HttpMethod method, String path) { + byte[] bytes = path.getBytes(StandardCharsets.UTF_8); + FastPathViews.RequestByteView pathView = new FastPathViews.RequestByteView(bytes, 0, bytes.length); + dev.relism.flash.models.RequestLine line = new dev.relism.flash.models.RequestLine( + method, pathView, null, + new FastPathViews.RequestByteView("HTTP/1.1".getBytes(StandardCharsets.UTF_8), 0, 8), + new dev.relism.flash.models.HeaderMap() + ); + return new Request(line, new byte[0]); + } + + // ── byteCompare_*: the direct EX-04 mechanism, in isolation ───────────── + + private FastPathViews.RequestByteView cmpView; + private byte[] cmpOther; + + @Setup(Level.Trial) + public void setupByteCompare() { + byte[] content = "/api/v1/products/electronics/00012345".getBytes(StandardCharsets.US_ASCII); + cmpView = new FastPathViews.RequestByteView(content, 0, content.length); + cmpOther = content.clone(); + } + + @Benchmark + public boolean byteCompare_longPath() { + return ByteCompare.equals(cmpView, 0, cmpOther, 0, cmpOther.length, true); + } + + @Benchmark + public boolean byteCompare_byteAtATime() { + return ByteCompare.equals(cmpView, 0, cmpOther, 0, cmpOther.length, false); + } +} diff --git a/flash/src/main/java/dev/relism/flash/RequestParser.java b/flash/src/main/java/dev/relism/flash/RequestParser.java index dcfaa76..76731de 100644 --- a/flash/src/main/java/dev/relism/flash/RequestParser.java +++ b/flash/src/main/java/dev/relism/flash/RequestParser.java @@ -1,5 +1,6 @@ package dev.relism.flash; +import dev.relism.flash.bytes.ByteScan; import dev.relism.flash.exceptions.MalformedRequestException; import dev.relism.flash.http.Http1Limits; import dev.relism.flash.http.HttpMethod; @@ -52,26 +53,6 @@ import java.util.Arrays; public class RequestParser { private static final int INITIAL_BUFFER_SIZE = 8192; - /** - * RFC 9110 §5.6.2 {@code tchar} set, table-driven so header-name validation is a single - * array read per byte rather than a chain of comparisons (R4/R5). Indexed directly by - * byte value; only defined for the ASCII range a valid header name can ever occupy. - */ - private static final boolean[] TCHAR = new boolean[128]; - - static { - for (byte b : "!#$%&'*+-.^_`|~".getBytes(java.nio.charset.StandardCharsets.US_ASCII)) { - TCHAR[b] = true; - } - for (char c = '0'; c <= '9'; c++) TCHAR[c] = true; - for (char c = 'A'; c <= 'Z'; c++) TCHAR[c] = true; - for (char c = 'a'; c <= 'z'; c++) TCHAR[c] = true; - } - - private static boolean isTChar(byte b) { - return b >= 0 && b < 128 && TCHAR[b]; - } - private final int maxHeaderBufferSize; private final InetSocketAddress remoteAddress; private final SSLSocket sslSocket; @@ -132,7 +113,7 @@ public class RequestParser { bufBase = 0; bufLen = 0; - int headerEndIdx = totalRead > 0 ? findEndOfHeader(buffer, base, base + totalRead) : -1; + int headerEndIdx = totalRead > 0 ? ByteScan.indexOfCrLfCrLf(buffer, base, base + totalRead) : -1; while (headerEndIdx == -1) { if (base + totalRead == buffer.length) { if (base > 0) { @@ -151,7 +132,7 @@ public class RequestParser { if (n <= 0) break; int prevTotal = totalRead; totalRead += n; - headerEndIdx = findEndOfHeader(buffer, base + Math.max(0, prevTotal - 3), base + totalRead); + headerEndIdx = ByteScan.indexOfCrLfCrLf(buffer, base + Math.max(0, prevTotal - 3), base + totalRead); } if (totalRead <= 0) return null; if (headerEndIdx == -1) { @@ -161,7 +142,7 @@ public class RequestParser { // ── Request line ───────────────────────────────────────────────────── - int methodEnd = find(buffer, base, headerEndIdx, (byte) ' '); + int methodEnd = ByteScan.indexOf(buffer, base, headerEndIdx, (byte) ' '); if (methodEnd == -1) throw new MalformedRequestException(400, "Invalid request line (method)"); if (methodEnd == base) throw new MalformedRequestException(400, "Missing HTTP method"); @@ -169,10 +150,10 @@ public class RequestParser { if (method == null) throw new MalformedRequestException(501, "Unsupported HTTP method"); int pathStart = methodEnd + 1; - int pathEnd = find(buffer, pathStart, headerEndIdx, (byte) ' '); + int pathEnd = ByteScan.indexOf(buffer, pathStart, headerEndIdx, (byte) ' '); if (pathEnd == -1) throw new MalformedRequestException(400, "Invalid request line (path)"); - int queryMark = find(buffer, pathStart, pathEnd, (byte) '?'); + int queryMark = ByteScan.indexOf(buffer, pathStart, pathEnd, (byte) '?'); FastPathViews.RequestByteView pathView = new FastPathViews.RequestByteView(buffer, pathStart, queryMark != -1 ? queryMark - pathStart : pathEnd - pathStart); FastPathViews.RequestByteView queryView = queryMark != -1 @@ -180,7 +161,7 @@ public class RequestParser { : null; int protocolStart = pathEnd + 1; - int protocolEnd = find(buffer, protocolStart, headerEndIdx, (byte) '\r'); + int protocolEnd = ByteScan.indexOf(buffer, protocolStart, headerEndIdx, (byte) '\r'); if (protocolEnd == -1) throw new MalformedRequestException(400, "Invalid request line (protocol)"); // EX-08: the request line itself (method SP target SP version) is bounded separately @@ -195,7 +176,7 @@ public class RequestParser { // ── Headers ────────────────────────────────────────────────────────── - int sectionStart = find(buffer, protocolEnd, headerEndIdx, (byte) '\n') + 1; + int sectionStart = ByteScan.indexOf(buffer, protocolEnd, headerEndIdx, (byte) '\n') + 1; int current = sectionStart; long contentLength = -1; boolean contentLengthSeen = false; @@ -212,7 +193,7 @@ public class RequestParser { throw new MalformedRequestException(400, "Obsolete line folding is not supported"); } - int lineEnd = find(buffer, current, headerEndIdx + 1, (byte) '\r'); + int lineEnd = ByteScan.indexOf(buffer, current, headerEndIdx + 1, (byte) '\r'); if (lineEnd == -1 || lineEnd == current) break; // EX-18: verify the '\r' is immediately followed by '\n' instead of blindly @@ -228,7 +209,7 @@ public class RequestParser { throw new MalformedRequestException(431, "Too many headers"); } - int colon = find(buffer, current, lineEnd, (byte) ':'); + int colon = ByteScan.indexOf(buffer, current, lineEnd, (byte) ':'); if (colon == -1) { throw new MalformedRequestException(400, "Header line missing ':'"); } @@ -236,7 +217,7 @@ public class RequestParser { throw new MalformedRequestException(431, "Header name exceeds " + Http1Limits.MAX_HEADER_NAME_LENGTH + " bytes"); } for (int i = current; i < colon; i++) { - if (!isTChar(buffer[i])) { + if (!ByteScan.isTChar(buffer[i])) { throw new MalformedRequestException(400, "Invalid header name character"); } } @@ -247,7 +228,7 @@ public class RequestParser { throw new MalformedRequestException(431, "Header value exceeds " + Http1Limits.MAX_HEADER_VALUE_LENGTH + " bytes"); } - if (equalsIgnoreCase(buffer, current, colon, "content-length")) { + if (ByteScan.equalsIgnoreCaseAscii(buffer, current, colon, "content-length")) { // EX-03: strict, overflow-safe parsing — replaces the old digit-skipping // parseLong, which silently accepted "5abc" as 5 and "-1" as 1. long parsed = parseContentLengthStrict(buffer, valueStart, lineEnd); @@ -259,7 +240,7 @@ public class RequestParser { } contentLength = parsed; contentLengthSeen = true; - } else if (equalsIgnoreCase(buffer, current, colon, "transfer-encoding")) { + } else if (ByteScan.equalsIgnoreCaseAscii(buffer, current, colon, "transfer-encoding")) { transferEncodingSeen = true; // Correctness fix found while implementing EX-02 in this exact code path // (registered as EX-35): the old check required the WHOLE value to equal @@ -319,34 +300,6 @@ public class RequestParser { return Request.forParsed(requestLine, in, contentLength, buffer, bodyStart, preBufLen, remoteAddress, sslSocket); } - // ── Buffer scanning utilities (hot path — keep branch-free where possible) ── - - private static int findEndOfHeader(byte[] buf, int from, int len) { - for (int i = from; i <= len - 4; i++) { - if (buf[i] == '\r' && buf[i+1] == '\n' && buf[i+2] == '\r' && buf[i+3] == '\n') - return i; - } - return -1; - } - - private static int find(byte[] buf, int start, int end, byte target) { - for (int i = start; i < end; i++) { - if (buf[i] == target) return i; - } - return -1; - } - - private static boolean equalsIgnoreCase(byte[] buf, int start, int end, String target) { - int len = end - start; - if (len != target.length()) return false; - for (int i = 0; i < len; i++) { - byte b = buf[start + i]; - if (b >= 'A' && b <= 'Z') b += 32; - if (b != (byte) target.charAt(i)) return false; - } - return true; - } - /** * Strict, overflow-safe {@code Content-Length} parsing ({@code EX-03}). Rejects: an empty * value, any non-digit byte (including a leading {@code +}/{@code -}, which are not @@ -395,6 +348,6 @@ public class RequestParser { } int tokenStart = lastComma + 1; while (tokenStart < e && (buf[tokenStart] == ' ' || buf[tokenStart] == '\t')) tokenStart++; - return equalsIgnoreCase(buf, tokenStart, e, "chunked"); + return ByteScan.equalsIgnoreCaseAscii(buf, tokenStart, e, "chunked"); } } diff --git a/flash/src/main/java/dev/relism/flash/bytes/ArrayBackedByteView.java b/flash/src/main/java/dev/relism/flash/bytes/ArrayBackedByteView.java new file mode 100644 index 0000000..8aeec16 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/ArrayBackedByteView.java @@ -0,0 +1,37 @@ +package dev.relism.flash.bytes; + +import dev.relism.fpr.core.ByteView; + +/** + * Capability interface for a {@link ByteView} that is a contiguous slice of a single backing + * {@code byte[]} — as opposed to a {@link SegmentedByteView}, which spans several arrays and + * cannot expose a single {@code (array, offset)} pair. + * + *

Every array-backed view in this codebase implements this: {@code RequestByteView}, + * {@code SocketByteView}, {@code StringByteView} (all in + * {@code dev.relism.flash.routing.routers.fastpathrouter.FastPathViews}), and {@link PooledSlice}. + * {@code MethodPathByteView} deliberately does not — it is a composite of a {@code byte[]} + * (method) and another {@link ByteView} (path), so it has no single backing array. + * + *

What this enables

+ * Anywhere code holds a plain {@link ByteView} and wants the fast path when the concrete + * instance happens to be array-backed, an {@code instanceof ArrayBackedByteView} check unlocks: + *
    + *
  • Single-allocation {@code String} construction — + * {@code new String(view.array(), view.offset(), view.length(), UTF_8)} instead of a + * byte-at-a-time copy into a scratch {@code byte[]} followed by a second allocation for + * the {@code String} itself ({@code EX-25}).
    • + *
  • A single {@code System.arraycopy} instead of a manual loop wherever a view's bytes need + * to be copied.
    • + *
+ * Code that only has a bare {@link ByteView} (e.g. because it received one across the + * {@link SegmentedByteView} boundary, from a future HPACK CONTINUATION-spanning block) keeps the + * byte-at-a-time fallback — this interface is an opportunistic fast path, never a requirement. + */ +public interface ArrayBackedByteView extends ByteView { + /** The backing array. Bytes {@code [offset(), offset() + length())} belong to this view. */ + byte[] array(); + + /** Offset of this view's first byte within {@link #array()}. */ + int offset(); +} diff --git a/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java b/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java new file mode 100644 index 0000000..e8c9dc2 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/ByteScan.java @@ -0,0 +1,331 @@ +package dev.relism.flash.bytes; + +import dev.relism.fpr.core.ByteView; + +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.nio.ByteOrder; + +/** + * The single home for protocol-neutral byte scanning: single-byte search, the four-byte + * {@code \r\n\r\n} header-terminator search (SWAR-accelerated), case-insensitive comparison, + * comma-separated token-list scanning ({@code Connection: a, b, c}), RFC 9110 {@code tchar} + * validation, and the case-insensitive header-name hash {@link dev.relism.flash.models.HeaderMap}'s + * index uses ({@code EX-09}). + * + *

Every method here is {@code static} and allocates nothing. Every SWAR method has a plain + * scalar counterpart ({@code *Scalar}) that exists for two reasons: it is what the tests use as + * the correctness oracle (property-tested against the SWAR version on randomized inputs — see + * {@code ByteScanTest}/{@code ByteScanFuzzTest}), and it is the documented fallback if a future + * measurement ever shows the SWAR path is not worth its complexity on some path (none has been + * found not worth it so far — see {@code DECISIONS.md} for the one path that {@em was} + * measured and kept, {@code EX-33}). + * + *

The SWAR technique used throughout

+ * Both {@link #indexOf} and {@link #indexOfCrLfCrLf} use the classic "does this word contain + * byte {@code b}" bit trick (Bit Twiddling Hacks, "Determine if a word has a byte equal to n"): + * XOR the 8-byte word against {@code b} broadcast into every lane (turning matching lanes to + * {@code 0x00}), then test for any zero lane with + * {@code (v - 0x0101010101010101L) & ~v & 0x8080808080808080L} — non-zero exactly when some lane + * was {@code 0x00} before the subtraction, i.e. some original lane equalled {@code b}. This finds + * *that a* matching lane exists in one word-sized read plus a handful of ALU ops, touching every + * byte only once per 8-byte stride in the common (no-match-yet) case, instead of once per byte. + * + *

Reading the word uses {@link MethodHandles#byteArrayViewVarHandle} with + * {@link ByteOrder#nativeOrder()} — deliberately native rather than a fixed order (contrast + * {@code fpr-core}'s {@code ByteCompare}, which fixes {@code LITTLE_ENDIAN} because it compares + * two independently-read words for bit-exact equality and so needs a byte order both reads + * agree on; nothing here compares across two separately-decoded words, so the fastest order for + * the host CPU is free to use). Byte-equality detection itself (finding that a matching lane + * exists in the mask) does not depend on which order was used to assemble the word — XOR and the + * haszero test are lane-wise operations, indifferent to how lanes map to memory offsets. + * Position extraction does depend on it: converting "which bit of the 64-bit mask is set" + * back into "which array index did that byte come from" requires knowing whether array byte 0 + * became the long's least-significant byte (little-endian) or most-significant byte + * (big-endian) — {@link #laneIndexOf} branches on {@link #NATIVE_IS_LITTLE} once, at class-init + * time, precisely to get this right on either host. + */ +public final class ByteScan { + private ByteScan() {} + + private static final ByteOrder NATIVE_ORDER = ByteOrder.nativeOrder(); + private static final boolean NATIVE_IS_LITTLE = NATIVE_ORDER == ByteOrder.LITTLE_ENDIAN; + private static final VarHandle LONG_VIEW = + MethodHandles.byteArrayViewVarHandle(long[].class, NATIVE_ORDER); + + private static final long LANE_LSB = 0x0101010101010101L; + private static final long LANE_MSB = 0x8080808080808080L; + + // ── tchar (RFC 9110 §5.6.2) ────────────────────────────────────────────── + + /** + * RFC 9110 §5.6.2 {@code tchar} set, table-driven so validation is a single array read per + * byte (R4/R5) rather than a chain of range comparisons. Indexed directly by byte value; + * only the ASCII range a valid header-name character can ever occupy is populated. + */ + private static final boolean[] TCHAR = new boolean[128]; + + static { + for (byte b : "!#$%&'*+-.^_`|~".getBytes(java.nio.charset.StandardCharsets.US_ASCII)) { + TCHAR[b] = true; + } + for (char c = '0'; c <= '9'; c++) TCHAR[c] = true; + for (char c = 'A'; c <= 'Z'; c++) TCHAR[c] = true; + for (char c = 'a'; c <= 'z'; c++) TCHAR[c] = true; + } + + /** Whether {@code b} is a valid RFC 9110 §5.6.2 {@code tchar} (a legal header-name byte). */ + public static boolean isTChar(byte b) { + return b >= 0 && b < 128 && TCHAR[b]; + } + + // ── Single-byte search ─────────────────────────────────────────────────── + + /** + * Index of the first occurrence of {@code target} in {@code buf[from, to)}, or {@code -1}. + * SWAR-accelerated: touches 8 bytes per word while no match has been found, falling back to + * a byte-at-a-time tail once fewer than 8 bytes remain. + */ + public static int indexOf(byte[] buf, int from, int to, byte target) { + long broadcast = (target & 0xFFL) * LANE_LSB; + int i = from; + while (i + 8 <= to) { + long word = (long) LONG_VIEW.get(buf, i); + long masked = hasZeroLane(word ^ broadcast); + if (masked != 0) { + return i + laneIndexOf(masked); + } + i += 8; + } + for (; i < to; i++) { + if (buf[i] == target) return i; + } + return -1; + } + + /** Plain byte-at-a-time reference implementation of {@link #indexOf} — the test oracle. */ + static int indexOfScalar(byte[] buf, int from, int to, byte target) { + for (int i = from; i < to; i++) { + if (buf[i] == target) return i; + } + return -1; + } + + // ── \r\n\r\n header terminator search ──────────────────────────────────── + + private static final byte CR = '\r', LF = '\n'; + + /** + * Index of the first {@code "\r\n\r\n"} in {@code buf[from, to)}, or {@code -1}. SWAR + * pre-filter (find a candidate {@code CR} byte 8 at a time) plus a cheap scalar 3-byte + * verify at each candidate — see the class Javadoc for the technique and + * {@code RequestParser}, {@code EX-33}, for why this replaced a fully byte-at-a-time scan. + */ + public static int indexOfCrLfCrLf(byte[] buf, int from, int to) { + int limit = to - 4; // last index at which a 4-byte match can start + int i = from; + while (i + 8 <= to) { + long word = (long) LONG_VIEW.get(buf, i); + long masked = hasZeroLane(word ^ CR_BROADCAST); + if (masked == 0) { + i += 8; + continue; + } + int crPos = i + laneIndexOf(masked); + if (crPos > limit) { + // Nearest CR candidate in this word can't fit a full match before `to`; no CR + // exists before it in [i, crPos) (laneIndexOf always finds the lowest-address + // match first), so nothing in [i, crPos) can match either — the scalar tail + // below, bounded by `limit`, correctly finds nothing without re-deriving that. + break; + } + if (buf[crPos + 1] == LF && buf[crPos + 2] == CR && buf[crPos + 3] == LF) { + return crPos; + } + i = crPos + 1; + } + for (; i <= limit; i++) { + if (buf[i] == CR && buf[i + 1] == LF && buf[i + 2] == CR && buf[i + 3] == LF) { + return i; + } + } + return -1; + } + + private static final long CR_BROADCAST = (CR & 0xFFL) * LANE_LSB; + + /** Plain byte-at-a-time reference implementation of {@link #indexOfCrLfCrLf} — the test oracle. */ + static int indexOfCrLfCrLfScalar(byte[] buf, int from, int to) { + for (int i = from; i <= to - 4; i++) { + if (buf[i] == CR && buf[i + 1] == LF && buf[i + 2] == CR && buf[i + 3] == LF) { + return i; + } + } + return -1; + } + + /** "Determine if a word has a byte equal to n" (Bit Twiddling Hacks), applied to {@code xored}. */ + private static long hasZeroLane(long xored) { + return (xored - LANE_LSB) & ~xored & LANE_MSB; + } + + /** Converts a {@link #hasZeroLane} result into the array-index offset of its lowest matching lane. */ + private static int laneIndexOf(long masked) { + return NATIVE_IS_LITTLE + ? Long.numberOfTrailingZeros(masked) >>> 3 + : 7 - (Long.numberOfLeadingZeros(masked) >>> 3); + } + + // ── Case-insensitive comparison ────────────────────────────────────────── + + private static byte foldAsciiUpper(byte b) { + return (b >= 'A' && b <= 'Z') ? (byte) (b + 32) : b; + } + + /** Case-insensitive (ASCII) equality of {@code buf[start, end)} against {@code target}. */ + public static boolean equalsIgnoreCaseAscii(byte[] buf, int start, int end, String target) { + int len = end - start; + if (len != target.length()) return false; + for (int i = 0; i < len; i++) { + if (foldAsciiUpper(buf[start + i]) != foldAsciiUpper((byte) target.charAt(i))) return false; + } + return true; + } + + /** Case-insensitive (ASCII) equality of two byte-array ranges. */ + public static boolean equalsIgnoreCaseAscii(byte[] a, int aStart, int aLen, byte[] b, int bStart, int bLen) { + if (aLen != bLen) return false; + for (int i = 0; i < aLen; i++) { + if (foldAsciiUpper(a[aStart + i]) != foldAsciiUpper(b[bStart + i])) return false; + } + return true; + } + + /** Case-insensitive (ASCII) equality of {@code view[start, end)} against {@code target}. */ + public static boolean equalsIgnoreCase(ByteView view, int start, int end, String target) { + int len = end - start; + if (len != target.length()) return false; + for (int i = 0; i < len; i++) { + if (foldAsciiUpper(view.byteAt(start + i)) != foldAsciiUpper((byte) target.charAt(i))) return false; + } + return true; + } + + // ── Comma-separated token lists (e.g. `Connection: keep-alive, Upgrade`) ──── + + /** + * Whether the comma-separated, OWS-tolerant token list {@code view} contains {@code token} + * (case-insensitive). The shared scanner behind both {@code Http1KeepAlive.isKeepAlive} and + * the {@code Connection: Upgrade} check ({@code EX-13}) — a single home so the two can never + * drift apart the way a whole-value {@code equals} check once did. + */ + public static boolean tokenListContains(ByteView view, String token) { + int len = view.length(), i = 0; + while (i < len) { + while (i < len && view.byteAt(i) == ' ') i++; + int start = i; + while (i < len && view.byteAt(i) != ',') i++; + if (tokenEqualsIgnoreCase(view, start, i, token)) return true; + i++; + } + return false; + } + + /** Case-insensitive compare of {@code view[start, end)}, trimming trailing spaces, against {@code token}. */ + public static boolean tokenEqualsIgnoreCase(ByteView view, int start, int end, String token) { + int wlen = end - start; + while (wlen > 0 && view.byteAt(start + wlen - 1) == ' ') wlen--; + return equalsIgnoreCase(view, start, start + wlen, token); + } + + // ── Header-name hash (EX-09) ───────────────────────────────────────────── + + /** + * Case-insensitive (ASCII fold) 32-bit FNV-1a hash of {@code buf[start, start + len)}. Used + * by {@link dev.relism.flash.models.HeaderMap}'s per-request index to compare a cheap hash + * before falling back to a full case-insensitive {@code memcmp}-equivalent + * ({@link #equalsIgnoreCaseAscii}) — two header names that differ anywhere hash differently + * with overwhelming probability, so the common "not the header I'm looking for" case resolves + * in one hash compare instead of a byte-by-byte scan. + */ + public static int hashNameIgnoreCaseAscii(byte[] buf, int start, int len) { + int hash = 0x811C9DC5; // FNV-1a 32-bit offset basis + for (int i = 0; i < len; i++) { + hash ^= (foldAsciiUpper(buf[start + i]) & 0xFF); + hash *= 0x01000193; // FNV-1a 32-bit prime + } + return hash; + } + + /** + * Same hash as {@link #hashNameIgnoreCaseAscii(byte[], int, int)}, computed directly from a + * lookup-key {@code String} (e.g. {@code "Content-Type"}) instead of already-scanned bytes — + * the two must agree bit-for-bit on equivalent ASCII content for + * {@link dev.relism.flash.models.HeaderMap}'s index (hash the request-declared bytes once at + * {@code reset()}; hash the caller's lookup key once per {@code first()}/{@code all()} call; + * compare the two cheap hashes before ever touching a full case-insensitive comparison). + */ + public static int hashNameIgnoreCaseAscii(String name) { + int hash = 0x811C9DC5; + int len = name.length(); + for (int i = 0; i < len; i++) { + hash ^= (foldAsciiUpper((byte) name.charAt(i)) & 0xFF); + hash *= 0x01000193; + } + return hash; + } + + // ── Decimal / hex parsing ──────────────────────────────────────────────── + + /** Sentinel returned by {@link #parseDecimalStrict} on any malformed or out-of-range input. */ + public static final long PARSE_INVALID = -1L; + + /** + * Strict, overflow-safe unsigned decimal parse of {@code buf[start, end)}: rejects an empty + * range, any non-{@code '0'..'9'} byte, more than 19 digits, and arithmetic overflow past + * {@link Long#MAX_VALUE}. Returns {@link #PARSE_INVALID} rather than throwing — the same + * shape {@code RequestParser}'s own {@code Content-Length} parser already hand-rolls (kept + * separate there since it also needs to throw a specific, differently-worded + * {@code MalformedRequestException} per failure mode); this is the general-purpose version + * for callers (HPACK integer decoding, frame-length fields) that just need a valid/invalid + * signal. + */ + public static long parseDecimalStrict(byte[] buf, int start, int end) { + int len = end - start; + if (len == 0 || len > 19) return PARSE_INVALID; + long value = 0; + for (int i = start; i < end; i++) { + byte c = buf[i]; + if (c < '0' || c > '9') return PARSE_INVALID; + int digit = c - '0'; + if (value > (Long.MAX_VALUE - digit) / 10) return PARSE_INVALID; + value = value * 10 + digit; + } + return value; + } + + /** + * Parses up to {@code maxDigits} hex digits (ASCII, either case) from {@code buf[start, end)} + * as an unsigned value. Returns {@link #PARSE_INVALID} if the range is empty, contains a + * non-hex-digit byte, or would need more than {@code maxDigits} digits to represent (the + * caller's bound against, e.g., a chunk-size line with an implausible number of digits). + */ + public static long parseHexStrict(byte[] buf, int start, int end, int maxDigits) { + int len = end - start; + if (len == 0 || len > maxDigits) return PARSE_INVALID; + long value = 0; + for (int i = start; i < end; i++) { + int digit = hexDigit(buf[i]); + if (digit < 0) return PARSE_INVALID; + value = (value << 4) | digit; + } + return value; + } + + private static int hexDigit(byte b) { + if (b >= '0' && b <= '9') return b - '0'; + if (b >= 'a' && b <= 'f') return b - 'a' + 10; + if (b >= 'A' && b <= 'F') return b - 'A' + 10; + return -1; + } +} diff --git a/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java b/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java new file mode 100644 index 0000000..25aaa05 --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/ByteWriter.java @@ -0,0 +1,154 @@ +package dev.relism.flash.bytes; + +import java.nio.charset.StandardCharsets; + +/** + * Index-based writer into a growable {@code byte[]} scratch buffer. Every {@code write*} method + * bounds-checks and grows the backing array only when the write would not otherwise fit — + * on an already-warm buffer (the steady-state case: the buffer has already grown to the + * connection's high-water mark), no method here allocates. + * + *

This is the infrastructure {@code EX-27} (Phase 6, collapsing {@code Http1ResponseWriter}'s + * ~10 small writes into one) and the Phase 5 frame layer serialize into: build a complete + * message into a {@code ByteWriter}-backed scratch buffer, then issue one bulk + * {@code write(buffer, 0, length())} — the same "serialize outside the lock, one bulk write" + * discipline {@link dev.relism.flash.h2.frame.Http2FrameWriter} already established for the h2 + * writer (see its Javadoc's "Layer 1"), extended to the byte layer both protocols share. + * + *

Lifetime and thread-safety contract

+ * Not thread-safe — exactly one writer at a time, matching every other per-connection scratch + * object in this codebase ({@code ConnectionScratch}, {@code HeaderMap}). {@link #reset()} + * repositions this writer to the start of its backing array for the next message; the backing + * array itself is never shrunk back down, only grown — the same amortized-to-zero-allocation + * growth policy {@code RequestParser}'s read buffer already uses. + */ +public final class ByteWriter { + private byte[] buf; + private int len; + + public ByteWriter(int initialCapacity) { + this.buf = new byte[Math.max(initialCapacity, 16)]; + } + + /** Repositions this writer to the start of its buffer, ready for the next message. */ + public void reset() { + len = 0; + } + + /** The backing buffer. Valid content is {@code [0, length())} — never assume {@code buf.length == length()}. */ + public byte[] array() { + return buf; + } + + /** How many bytes have been written since the last {@link #reset()}. */ + public int length() { + return len; + } + + private void ensure(int additional) { + int needed = len + additional; + if (needed <= buf.length) return; + int grown = buf.length * 2; + while (grown < needed) grown *= 2; + byte[] next = new byte[grown]; + System.arraycopy(buf, 0, next, 0, len); + buf = next; + } + + public void writeByte(byte b) { + ensure(1); + buf[len++] = b; + } + + public void writeBytes(byte[] src) { + writeBytes(src, 0, src.length); + } + + public void writeBytes(byte[] src, int off, int srcLen) { + ensure(srcLen); + System.arraycopy(src, off, buf, len, srcLen); + len += srcLen; + } + + /** + * Writes {@code value}'s ASCII decimal digits (no sign — callers write {@code '-'} via + * {@link #writeByte} first if needed). {@code value} must be non-negative. + */ + public void writeDecimal(long value) { + if (value < 0) throw new IllegalArgumentException("writeDecimal requires a non-negative value: " + value); + if (value == 0) { + writeByte((byte) '0'); + return; + } + // Digits emerge least-significant-first; stage them in a small fixed buffer (at most 20 + // digits for any long) and copy in reverse — avoids a second pass to compute digit count. + byte[] digits = new byte[20]; + int n = 0; + long v = value; + while (v > 0) { + digits[n++] = (byte) ('0' + (v % 10)); + v /= 10; + } + ensure(n); + for (int i = n - 1; i >= 0; i--) buf[len++] = digits[i]; + } + + private static final byte[] HEX_DIGITS = "0123456789abcdef".getBytes(StandardCharsets.US_ASCII); + + /** Writes {@code value}'s lowercase hex digits, no leading zeros (except for {@code value == 0}, which writes {@code "0"}). */ + public void writeHex(int value) { + if (value == 0) { + writeByte((byte) '0'); + return; + } + byte[] digits = new byte[8]; + int n = 0; + int v = value; + while (v != 0) { + digits[n++] = HEX_DIGITS[v & 0xF]; + v >>>= 4; + } + ensure(n); + for (int i = n - 1; i >= 0; i--) buf[len++] = digits[i]; + } + + /** Writes {@code s}'s ASCII bytes, lower-cased. {@code s} must be ASCII-only. */ + public void writeAsciiLower(String s) { + int n = s.length(); + ensure(n); + for (int i = 0; i < n; i++) { + char c = s.charAt(i); + if (c >= 'A' && c <= 'Z') c += 32; + buf[len++] = (byte) c; + } + } + + /** Big-endian 16-bit write — an HTTP/2 frame's stream-dependent fields, SETTINGS values, etc. */ + public void writeUInt16(int value) { + ensure(2); + buf[len++] = (byte) (value >>> 8); + buf[len++] = (byte) value; + } + + /** Big-endian 24-bit write — an HTTP/2 frame header's length field. */ + public void writeUInt24(int value) { + ensure(3); + buf[len++] = (byte) (value >>> 16); + buf[len++] = (byte) (value >>> 8); + buf[len++] = (byte) value; + } + + /** Big-endian 31-bit write (top bit always 0) — an HTTP/2 stream identifier. */ + public void writeUInt31(int value) { + writeUInt32(value & 0x7FFFFFFF); + } + + /** Big-endian 32-bit write — an HTTP/2 window-size increment, SETTINGS value, etc. */ + public void writeUInt32(int value) { + ensure(4); + buf[len++] = (byte) (value >>> 24); + buf[len++] = (byte) (value >>> 16); + buf[len++] = (byte) (value >>> 8); + buf[len++] = (byte) value; + } +} diff --git a/flash/src/main/java/dev/relism/flash/bytes/Pairs.java b/flash/src/main/java/dev/relism/flash/bytes/Pairs.java new file mode 100644 index 0000000..4552f2a --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/Pairs.java @@ -0,0 +1,42 @@ +package dev.relism.flash.bytes; + +/** + * The allocation-free idiom for returning two {@code int}s from a method without an object: + * pack both into one {@code long}, unpack at the call site. Already used, hand-rolled, in four + * places ({@code HeaderMap.findFirst}, {@code QueryParams.findFirst}, and others) before this + * class existed — this is the single named home for the shifts so they are not duplicated (and + * potentially inconsistently duplicated — e.g. one copy masking with {@code 0xFFFFFFFFL} and + * another forgetting to) five times over. + * + *

Why this works

+ * A {@code long} is 64 bits; each packed {@code int} is 32. {@link #pack} left-shifts the high + * half into the top 32 bits and OR's the low half into the bottom 32. {@link #lo} must mask with + * {@code 0xFFFFFFFFL} rather than simply cast to {@code int} after no mask, because a right-shift + * of a negative {@code long} sign-extends — the mask discards everything above bit 31 before the + * narrowing cast happens implicitly. {@link #hi} needs no mask: a right-shift by 32 already + * leaves only the original high bits in the low 32 positions of the result. + * + *

Encoding convention used across this codebase

+ * Every {@code findFirst}-shaped method in this codebase packs {@code (start << 32) | length}, + * i.e. {@code hi() == start} and {@code lo() == length}. {@code -1L} is the shared "not found" + * sentinel (a valid {@code (start, length)} pair can never be negative, since both halves are + * non-negative offsets/lengths). + */ +public final class Pairs { + private Pairs() {} + + /** Packs two {@code int}s into one {@code long}: {@code hi} in the upper 32 bits, {@code lo} in the lower 32. */ + public static long pack(int hi, int lo) { + return ((long) hi << 32) | (lo & 0xFFFFFFFFL); + } + + /** Extracts the upper 32 bits packed by {@link #pack}. */ + public static int hi(long packed) { + return (int) (packed >> 32); + } + + /** Extracts the lower 32 bits packed by {@link #pack}. */ + public static int lo(long packed) { + return (int) (packed & 0xFFFFFFFFL); + } +} diff --git a/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java b/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java new file mode 100644 index 0000000..09c5e3c --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/PooledSlice.java @@ -0,0 +1,53 @@ +package dev.relism.flash.bytes; + +/** + * A mutable, reusable {@link ArrayBackedByteView} — the {@code EX-05} fix. Replaces the + * per-call {@code new ByteView() { ... }} anonymous-class allocation that used to live in + * {@code HeaderMap.view}, {@code QueryParams.view}, and {@code PathParams.view}: instead of + * allocating a fresh view object (plus its capturing instance) on every call, a small + * {@link SlicePool} of these hands out an existing instance, repositioned in place. + * + *

Lifetime contract

+ * A {@code PooledSlice} handed out by {@link SlicePool#acquire} is valid only until the pool + * wraps around and reuses the same slot — see {@link SlicePool}'s own Javadoc for the exact + * "valid until the Nth subsequent acquire, or end of request" rule the owning class (e.g. + * {@code HeaderMap}) documents precisely for its own {@code view()} method. Never retain a + * {@code PooledSlice} past that window, for the same reason the old anonymous view could not be + * retained past the handler: the bytes (and, here, additionally the slice object itself) are + * about to be repositioned out from under a stale reference. + */ +public final class PooledSlice implements ArrayBackedByteView { + private byte[] array; + private int offset; + private int length; + + /** Repositions this slice over {@code array[offset, offset + length)}. Zero allocation. */ + public void reset(byte[] array, int offset, int length) { + this.array = array; + this.offset = offset; + this.length = length; + } + + @Override + public byte[] array() { + return array; + } + + @Override + public int offset() { + return offset; + } + + @Override + public int length() { + return length; + } + + @Override + public byte byteAt(int index) { + if (index < 0 || index >= length) { + throw new IndexOutOfBoundsException("Index " + index + " out of bounds for length " + length); + } + return array[offset + index]; + } +} diff --git a/flash/src/main/java/dev/relism/flash/bytes/SegmentedByteView.java b/flash/src/main/java/dev/relism/flash/bytes/SegmentedByteView.java new file mode 100644 index 0000000..459bcee --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/SegmentedByteView.java @@ -0,0 +1,81 @@ +package dev.relism.flash.bytes; + +import dev.relism.fpr.core.ByteView; + +/** + * A {@link ByteView} over up to {@code K} discontiguous {@code byte[]} segments, presented as one + * logical byte sequence. Exists for the one case in this codebase where a "single contiguous + * slice of one buffer" model (every other {@link ByteView} implementation) does not hold: an + * HPACK header block whose encoding spans more than one {@code CONTINUATION} frame (RFC 9113 + * §6.10), where each frame's payload lives in its own connection-buffer region. + * + *

Deliberately not array-backed

+ * This does not implement {@link ArrayBackedByteView} — there is no single {@code (array, + * offset)} pair that describes it — and {@link #supportsLong()} returns {@code false} + * unconditionally rather than attempting a cross-segment 8-byte read ({@code EX-04}'s word-at-a- + * time path is only sound for a genuinely contiguous backing array; see + * {@code FastPathViews.MethodPathByteView} for the other deliberately-segmented view in this + * codebase, which makes the same choice for the same reason). + * + *

Reusable, not allocated per block

+ * {@link #reset} repositions this view over a new set of segments without allocating — the same + * idiom {@link PooledSlice} uses for the contiguous case. The {@code segments}/{@code offsets}/ + * {@code lengths} arrays passed to {@link #reset} are retained by reference, not copied; the + * caller owns their lifetime (typically the connection's HPACK scratch, sized to + * {@code Http2Limits.MAX_CONTINUATION_FRAMES_PER_BLOCK}). + * + *

Cost model

+ * {@link #byteAt} walks the segment table to find which segment an index falls in — O(segments), + * not O(1) — because this view exists precisely for the rare, deliberately-bounded case + * (at most {@code MAX_CONTINUATION_FRAMES_PER_BLOCK} segments); optimizing it further would add + * complexity for a path that, by construction, is never hot. + */ +public final class SegmentedByteView implements ByteView { + private byte[][] segments; + private int[] offsets; + private int[] lengths; + private int count; + private int totalLength; + + /** + * Repositions this view over {@code segments[0..count)}, where segment {@code i} contributes + * bytes {@code segments[i][offsets[i], offsets[i] + lengths[i])}. Zero allocation: the three + * arrays are retained by reference. + */ + public void reset(byte[][] segments, int[] offsets, int[] lengths, int count) { + this.segments = segments; + this.offsets = offsets; + this.lengths = lengths; + this.count = count; + int total = 0; + for (int i = 0; i < count; i++) total += lengths[i]; + this.totalLength = total; + } + + @Override + public int length() { + return totalLength; + } + + @Override + public byte byteAt(int index) { + if (index < 0 || index >= totalLength) { + throw new IndexOutOfBoundsException("Index " + index + " out of bounds for length " + totalLength); + } + int remaining = index; + for (int i = 0; i < count; i++) { + int len = lengths[i]; + if (remaining < len) { + return segments[i][offsets[i] + remaining]; + } + remaining -= len; + } + throw new IndexOutOfBoundsException("Index " + index + " out of bounds for length " + totalLength); + } + + /** Always {@code false} — see the class Javadoc for why a cross-segment word read is unsound. */ + @Override + public boolean supportsLong() { + return false; + } +} diff --git a/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java b/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java new file mode 100644 index 0000000..eff42bc --- /dev/null +++ b/flash/src/main/java/dev/relism/flash/bytes/SlicePool.java @@ -0,0 +1,52 @@ +package dev.relism.flash.bytes; + +/** + * A small, fixed-size ring of {@link PooledSlice} instances — one per {@code ConnectionScratch}- + * held call site that used to allocate a fresh {@code ByteView} per call ({@code EX-05}: + * {@code HeaderMap.view}, {@code QueryParams.view}, {@code PathParams.view}). + * + *

Why a ring, not a single reused slice

+ * A single reused slice (the shape {@code HeaderMap.forEach} already uses for its two + * {@code nameSlice}/{@code valueSlice} fields) is correct only when the caller is guaranteed to + * finish with one slice before the next is produced — true for a single {@code forEach} callback + * invocation, false for {@code view()}: a handler might reasonably call + * {@code headers.view("A")} and {@code headers.view("B")} and want to compare both. A ring of + * {@code size} slices lets up to {@code size} calls' results stay simultaneously valid. + * + *

Lifetime contract

+ * A slice returned by {@link #acquire} is valid until either the request ends, or {@link #acquire} + * is called {@code size} more times on the same pool (at which point the ring has wrapped around + * and repositioned that same slot for a new caller) — whichever comes first. This must be + * restated precisely on every method that hands out a slice from a pool (see + * {@code HeaderMap.view}'s Javadoc for the canonical wording); it is a real, testable hazard, not + * a hypothetical one — see {@code SlicePoolTest#wraparoundAliasesThePreviouslyReturnedSlice} for + * a demonstration. + */ +public final class SlicePool { + private final PooledSlice[] slices; + private int next = 0; + + /** A ring of {@code size} reusable slices. {@code size} must be at least 1. */ + public SlicePool(int size) { + if (size < 1) throw new IllegalArgumentException("SlicePool size must be at least 1: " + size); + slices = new PooledSlice[size]; + for (int i = 0; i < size; i++) slices[i] = new PooledSlice(); + } + + /** How many slices this pool cycles through before a caller's slice is reused. */ + public int size() { + return slices.length; + } + + /** + * Returns the next slice in the ring, repositioned over {@code array[offset, offset + length)}. + * Zero allocation — the returned instance already existed. + */ + public PooledSlice acquire(byte[] array, int offset, int length) { + PooledSlice slice = slices[next]; + next++; + if (next == slices.length) next = 0; + slice.reset(array, offset, length); + return slice; + } +} diff --git a/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java b/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java index 43cc581..f24eced 100644 --- a/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java +++ b/flash/src/main/java/dev/relism/flash/http1/Http1Connection.java @@ -39,6 +39,11 @@ public final class Http1Connection implements ConnectionProtocol { OutputStream out = ctx.out(); byte[] idleProbe = new byte[1]; + // EX-06 (router half): created once per connection, exactly like `parser` above, and + // reused across every request on this connection — see AbstractRouter#newScratch. + Object routeScratch = ctx.router().newScratch(); + Object wsRouteScratch = ctx.wsRouter().newScratch(); + while (!ctx.stopped().getAsBoolean()) { // EX-07: wait for the next request to begin, bounded by the generous // idle-keep-alive timeout — sitting idle between keep-alive requests is normal, not @@ -77,7 +82,7 @@ public final class Http1Connection implements ConnectionProtocol { if (request.method() == HttpMethod.GET && WebSocketUpgrade.isWebSocketUpgrade(request)) { in.clearDeadline(); // the WS session loop is long-lived; it paces itself - WebSocketHandler wsHandler = ctx.wsRouter().route(request); + WebSocketHandler wsHandler = ctx.wsRouter().route(request, wsRouteScratch); if (wsHandler == null) { out.write(WebSocketUpgrade.REJECT_400); out.flush(); @@ -103,7 +108,7 @@ public final class Http1Connection implements ConnectionProtocol { boolean keepAlive = Http1KeepAlive.isKeepAlive(request); Response response = new Response(200, ContentType.TEXT_PLAIN); - RequestHandler handler = ctx.router().route(request); + RequestHandler handler = ctx.router().route(request, routeScratch); if (handler == null) handler = ctx.router().getNotFoundHandler(); try { diff --git a/flash/src/main/java/dev/relism/flash/models/HeaderMap.java b/flash/src/main/java/dev/relism/flash/models/HeaderMap.java index 090d6ec..a8e105f 100644 --- a/flash/src/main/java/dev/relism/flash/models/HeaderMap.java +++ b/flash/src/main/java/dev/relism/flash/models/HeaderMap.java @@ -1,10 +1,14 @@ package dev.relism.flash.models; +import dev.relism.flash.bytes.ByteScan; +import dev.relism.flash.bytes.SlicePool; +import dev.relism.flash.http.Http1Limits; import dev.relism.fpr.core.ByteView; import lombok.NoArgsConstructor; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** @@ -23,31 +27,99 @@ import java.util.List; * values retrieved via {@link #first}/{@link #all} are safe (they are independent * heap copies); the {@code HeaderMap} object itself is not. *
  • {@link #view} returns a zero-copy {@link dev.relism.fpr.core.ByteView} slice - * into the live buffer. Storing this view and reading it after the handler - * returns (e.g. in an async callback, a {@link java.util.concurrent.CompletableFuture} + * into the live buffer, drawn from a small {@link SlicePool} (see {@link #view}'s own + * Javadoc for the exact reuse window). Storing this view and reading it after the + * handler returns (e.g. in an async callback, a {@link java.util.concurrent.CompletableFuture} * continuation, or a virtual-thread handoff) is a data race — the bytes * may have been overwritten by the next request. Copy to a {@code String} or * {@code byte[]} before leaving the synchronous handler scope.
    • * + * + *

    {@code EX-09}: an index built once per {@link #reset}, not rescanned per lookup

    + * {@link #reset} scans the header section exactly once and records, per header, its name/value + * byte offsets and a case-insensitive 32-bit hash of the name — into {@code int[]} arrays grown + * (never shrunk) to this connection's high-water mark. Every lookup method + * ({@link #first}, {@link #all}, {@link #view}, {@link #valueEqualsIgnoreCase}) then walks that + * small index instead of rescanning raw bytes: a hash compare (cheap) before ever falling back to + * a full case-insensitive name comparison. A realistic middleware chain performs 6–10 lookups per + * request (OIDC reads {@code Authorization}/{@code Cookie}, the limiter reads + * {@code X-Forwarded-For}, CORS reads {@code Origin}, keep-alive reads {@code Connection}); before + * this, each of those rescanned the entire header block from scratch — O(n·m). Now the header + * section is scanned once regardless of how many lookups follow — strictly less total work even + * for a single lookup, and asymptotically better for the realistic multi-lookup case. */ @NoArgsConstructor public class HeaderMap { + private static final int INITIAL_INDEX_CAPACITY = 16; + private static final int VIEW_POOL_SIZE = 4; + private byte[] buffer; private int sectionStart; private int sectionEnd; - // Lazily created, then reused for the life of this HeaderMap (i.e. the connection — - // see the class javadoc) across every #forEach call and every header within a call. - // Same idiom as #view's per-call anonymous ByteView, just amortized to zero allocations - // instead of two per header: the slices are repositioned in place, not reallocated. + // EX-09 index — grown (never shrunk) to this connection's high-water mark, rebuilt in place + // by every reset() call. Entry i's name is buffer[nameOffsets[i], nameOffsets[i]+nameLengths[i]), + // its value is buffer[valueOffsets[i], valueOffsets[i]+valueLengths[i]). + private int headerCount; + private int[] nameOffsets = new int[INITIAL_INDEX_CAPACITY]; + private int[] nameLengths = new int[INITIAL_INDEX_CAPACITY]; + private int[] valueOffsets = new int[INITIAL_INDEX_CAPACITY]; + private int[] valueLengths = new int[INITIAL_INDEX_CAPACITY]; + private int[] nameHashes = new int[INITIAL_INDEX_CAPACITY]; + + // EX-05: pooled, reused slices for view() — see its own Javadoc for the reuse window. + private final SlicePool viewPool = new SlicePool(VIEW_POOL_SIZE); + + // forEach's own pair, reused across every header of every call — same idiom as viewPool, + // just a fixed pair rather than a ring, since forEach's contract only needs one name/value + // pair valid at a time (see forEach's Javadoc). private Slice nameSlice; private Slice valueSlice; - /** Resets this map to the header section {@code buffer[sectionStart, sectionEnd)}. */ + /** Resets this map to the header section {@code buffer[sectionStart, sectionEnd)}, rebuilding the {@code EX-09} index. */ public void reset(byte[] buffer, int sectionStart, int sectionEnd) { this.buffer = buffer; this.sectionStart = sectionStart; this.sectionEnd = sectionEnd; + buildIndex(); + } + + private void buildIndex() { + headerCount = 0; + if (buffer == null) return; + int i = sectionStart; + while (i < sectionEnd) { + int lineEnd = findCR(i); + int colon = findColon(i, lineEnd); + if (colon != -1) { + int vs = skipSpaces(colon + 1, lineEnd); + ensureIndexCapacity(headerCount + 1); + nameOffsets[headerCount] = i; + nameLengths[headerCount] = colon - i; + valueOffsets[headerCount] = vs; + valueLengths[headerCount] = lineEnd - vs; + nameHashes[headerCount] = ByteScan.hashNameIgnoreCaseAscii(buffer, i, colon - i); + headerCount++; + } + i = lineEnd + 2; + } + } + + private void ensureIndexCapacity(int needed) { + if (needed <= nameOffsets.length) return; + // EX-08 (Http1Limits.MAX_HEADER_COUNT) already rejects any request with more headers + // than this before it ever reaches reset() — this can only fire while growing toward + // that ceiling, never past it. Asserted, not silently truncated: an index that silently + // dropped headers past this point would be a correctness bug, not a capacity one. + assert needed <= Http1Limits.MAX_HEADER_COUNT + : "header count " + needed + " exceeds Http1Limits.MAX_HEADER_COUNT — RequestParser should have rejected this already"; + int grown = nameOffsets.length; + while (grown < needed) grown *= 2; + nameOffsets = Arrays.copyOf(nameOffsets, grown); + nameLengths = Arrays.copyOf(nameLengths, grown); + valueOffsets = Arrays.copyOf(valueOffsets, grown); + valueLengths = Arrays.copyOf(valueLengths, grown); + nameHashes = Arrays.copyOf(nameHashes, grown); } /** @@ -69,19 +141,12 @@ public class HeaderMap { nameSlice = new Slice(); valueSlice = new Slice(); } - int i = sectionStart; - while (i < sectionEnd) { - int lineEnd = findCR(i); - int colon = findColon(i, lineEnd); - if (colon != -1) { - int vs = skipSpaces(colon + 1, lineEnd); - nameSlice.start = i; - nameSlice.len = colon - i; - valueSlice.start = vs; - valueSlice.len = lineEnd - vs; - consumer.accept(nameSlice, valueSlice); - } - i = lineEnd + 2; + for (int i = 0; i < headerCount; i++) { + nameSlice.start = nameOffsets[i]; + nameSlice.len = nameLengths[i]; + valueSlice.start = valueOffsets[i]; + valueSlice.len = valueLengths[i]; + consumer.accept(nameSlice, valueSlice); } } @@ -108,26 +173,21 @@ public class HeaderMap { /** Returns the first value of header {@code name} (case-insensitive), or {@code null}. */ public String first(String name) { - long r = findFirst(name); - if (r < 0) return null; - int s = (int) (r >> 32), l = (int) (r & 0xFFFFFFFFL); - return new String(buffer, s, l, StandardCharsets.UTF_8); + int i = indexOfHeader(name); + if (i < 0) return null; + return new String(buffer, valueOffsets[i], valueLengths[i], StandardCharsets.UTF_8); } /** Returns all values of header {@code name} in declaration order, or an empty list. */ public List all(String name) { if (buffer == null) return List.of(); List result = null; - int i = sectionStart; - while (i < sectionEnd) { - int lineEnd = findCR(i); - int colon = findColon(i, lineEnd); - if (colon != -1 && keyMatches(i, colon - i, name)) { - int vs = skipSpaces(colon + 1, lineEnd); + int hash = ByteScan.hashNameIgnoreCaseAscii(name); + for (int i = 0; i < headerCount; i++) { + if (nameHashes[i] == hash && ByteScan.equalsIgnoreCaseAscii(buffer, nameOffsets[i], nameOffsets[i] + nameLengths[i], name)) { if (result == null) result = new ArrayList<>(); - result.add(new String(buffer, vs, lineEnd - vs, StandardCharsets.UTF_8)); + result.add(new String(buffer, valueOffsets[i], valueLengths[i], StandardCharsets.UTF_8)); } - i = lineEnd + 2; } return result != null ? result : List.of(); } @@ -135,61 +195,47 @@ public class HeaderMap { /** Returns all header values in declaration order. */ public List all() { if (buffer == null) return List.of(); - List result = new ArrayList<>(); - int i = sectionStart; - while (i < sectionEnd) { - int lineEnd = findCR(i); - int colon = findColon(i, lineEnd); - if (colon != -1) { - int vs = skipSpaces(colon + 1, lineEnd); - result.add(new String(buffer, vs, lineEnd - vs, StandardCharsets.UTF_8)); - } - i = lineEnd + 2; + List result = new ArrayList<>(headerCount); + for (int i = 0; i < headerCount; i++) { + result.add(new String(buffer, valueOffsets[i], valueLengths[i], StandardCharsets.UTF_8)); } return result; } /** Case-insensitive comparison of the first value of {@code name} against {@code value}. */ public boolean valueEqualsIgnoreCase(String name, String value) { - long r = findFirst(name); - if (r < 0) return false; - int vs = (int) (r >> 32), vl = (int) (r & 0xFFFFFFFFL); - if (vl != value.length()) return false; - for (int i = 0; i < vl; i++) { - byte b = buffer[vs + i]; - if (b >= 'A' && b <= 'Z') b += 32; - char c = value.charAt(i); - if (c >= 'A' && c <= 'Z') c += 32; - if (b != (byte) c) return false; - } - return true; + int i = indexOfHeader(name); + if (i < 0) return false; + return ByteScan.equalsIgnoreCaseAscii(buffer, valueOffsets[i], valueOffsets[i] + valueLengths[i], value); } - /** Returns a zero-copy {@link ByteView} over the first value of {@code name}, or {@code null}. */ + /** + * Returns a zero-copy {@link ByteView} over the first value of {@code name}, or {@code null}. + * + *

    {@code EX-05}: pooled, not allocated per call

    + * The returned view is drawn from a small internal {@link SlicePool} rather than allocated + * fresh. It stays valid until either the request ends, or {@link #view} is called + * {@value #VIEW_POOL_SIZE} more times on this same {@code HeaderMap} — whichever comes + * first — at which point the ring wraps around and silently repositions the same instance + * over different bytes. A handler that needs more than {@value #VIEW_POOL_SIZE} views alive + * at once should copy the earlier ones to {@code String}/{@code byte[]} before requesting more. + */ public ByteView view(String name) { - long r = findFirst(name); - if (r < 0) return null; - final int s = (int) (r >> 32), l = (int) (r & 0xFFFFFFFFL); - return new ByteView() { - public int length() { return l; } - public byte byteAt(int i) { return buffer[s + i]; } - }; + int i = indexOfHeader(name); + if (i < 0) return null; + return viewPool.acquire(buffer, valueOffsets[i], valueLengths[i]); } - /** Returns {@code (valStart << 32) | valLen}, or {@code -1} if not found. */ - private long findFirst(String name) { - if (buffer == null) return -1L; - int i = sectionStart; - while (i < sectionEnd) { - int lineEnd = findCR(i); - int colon = findColon(i, lineEnd); - if (colon != -1 && keyMatches(i, colon - i, name)) { - int vs = skipSpaces(colon + 1, lineEnd); - return ((long) vs << 32) | (lineEnd - vs); + /** Index into the {@code EX-09} arrays of the first header named {@code name}, or {@code -1}. */ + private int indexOfHeader(String name) { + if (buffer == null) return -1; + int hash = ByteScan.hashNameIgnoreCaseAscii(name); + for (int i = 0; i < headerCount; i++) { + if (nameHashes[i] == hash && ByteScan.equalsIgnoreCaseAscii(buffer, nameOffsets[i], nameOffsets[i] + nameLengths[i], name)) { + return i; } - i = lineEnd + 2; } - return -1L; + return -1; } private int findCR(int from) { @@ -206,16 +252,4 @@ public class HeaderMap { while (from < end && buffer[from] == ' ') from++; return from; } - - private boolean keyMatches(int start, int len, String name) { - if (len != name.length()) return false; - for (int i = 0; i < len; i++) { - byte b = buffer[start + i]; - if (b >= 'A' && b <= 'Z') b += 32; - char c = name.charAt(i); - if (c >= 'A' && c <= 'Z') c += 32; - if (b != (byte) c) return false; - } - return true; - } } diff --git a/flash/src/main/java/dev/relism/flash/models/PathParams.java b/flash/src/main/java/dev/relism/flash/models/PathParams.java index a97c156..751152b 100644 --- a/flash/src/main/java/dev/relism/flash/models/PathParams.java +++ b/flash/src/main/java/dev/relism/flash/models/PathParams.java @@ -1,5 +1,7 @@ package dev.relism.flash.models; +import dev.relism.flash.bytes.ArrayBackedByteView; +import dev.relism.flash.bytes.SlicePool; import dev.relism.flash.routing.AbstractRouter; import dev.relism.fpr.core.ByteView; @@ -7,19 +9,67 @@ import java.nio.charset.StandardCharsets; /** * Path parameters captured during routing, stored as byte offsets into the path view. - * {@link #get} allocates a String on call; {@link #view} is zero-copy. + * {@link #get} allocates a {@code String} on call (in one allocation when {@link #source} is + * {@link ArrayBackedByteView} — {@code EX-25} — two otherwise); {@link #view} is zero-copy. + * + *

    Reusable instances ({@code EX-19})

    + * The public constructor below builds a one-shot, fixed-size instance (used by + * {@code AbstractWsRouter} and by tests) — {@code names.length} is taken as the exact param + * count. {@code FastPathRouterImpl}'s per-connection scratch instead owns a single long-lived + * {@code PathParams} whose backing arrays are grown to the connection's high-water mark and + * never reallocated after warmup; because those arrays can be larger than the current request's + * actual param count, that path uses {@link #reset}, which — unlike the constructor — takes the + * live count explicitly rather than inferring it from array length. Both this constructor and + * {@link #reset} are {@code public} rather than package-private (matching + * {@link HeaderMap#reset}'s own precedent for a reusable buffer-backed object): the router + * implementation that owns the reusable instance lives in a different package + * ({@code dev.relism.flash.routing.routers.fastpathrouter}), and {@code PathParams.inject}'s + * own doc explains why this codebase prefers a small public surface here over a cross-package + * friend-access workaround. A {@code PathParams} obtained this way has the same "do not retain + * past the handler" lifetime contract as {@link HeaderMap}'s buffer-backed views: the next + * request on the same connection repositions the same arrays. */ public class PathParams { - private final ByteView source; + private static final int VIEW_POOL_SIZE = 4; + + private ByteView source; private final String[] names; private final int[] starts; private final int[] lens; + private int count; + + // EX-05: created lazily, only if view() is ever actually called. + private SlicePool viewPool; public PathParams(ByteView source, String[] names, int[] starts, int[] lens) { this.source = source; this.names = names; this.starts = starts; this.lens = lens; + this.count = names.length; + } + + /** + * Builds an instance meant only for {@link #reset}: no source yet, and {@code count} starts + * at 0 until the first {@link #reset} call. {@code names}/{@code starts}/{@code lens} may be + * larger than any single request's param count — see the class Javadoc. + */ + public PathParams(String[] names, int[] starts, int[] lens) { + this.source = null; + this.names = names; + this.starts = starts; + this.lens = lens; + this.count = 0; + } + + /** + * Repositions this instance over a new request: {@code count} (which may be less than + * {@code names.length} — see the class Javadoc) params are now valid, read out of the same + * backing arrays the constructor was given, against the new {@code source}. Zero allocation. + */ + public void reset(ByteView source, int count) { + this.source = source; + this.count = count; } /** @@ -34,23 +84,41 @@ public class PathParams { public String get(String name) { int i = indexOf(name); if (i < 0) return null; - byte[] bytes = new byte[lens[i]]; - for (int j = 0; j < lens[i]; j++) bytes[j] = source.byteAt(starts[i] + j); + int start = starts[i], len = lens[i]; + // EX-25: a single-copy String construction when the source is a contiguous array slice + // (always true for h1 today) instead of a byte-at-a-time copy into a scratch array + // followed by a second allocation for the String itself. + if (source instanceof ArrayBackedByteView abv) { + return new String(abv.array(), abv.offset() + start, len, StandardCharsets.UTF_8); + } + byte[] bytes = new byte[len]; + for (int j = 0; j < len; j++) bytes[j] = source.byteAt(start + j); return new String(bytes, StandardCharsets.UTF_8); } + /** + * Returns a zero-copy view over path param {@code name}, or {@code null}. {@code EX-05}: + * drawn from a small internal {@link SlicePool} when {@link #source} is array-backed (always + * true for h1 today) — same reuse-window contract as {@link HeaderMap#view}. Falls back to a + * fresh (allocating) view otherwise — never exercised on the real request path. + */ ByteView view(String name) { int i = indexOf(name); if (i < 0) return null; - final int s = starts[i], l = lens[i]; + int s = starts[i], l = lens[i]; + if (source instanceof ArrayBackedByteView abv) { + if (viewPool == null) viewPool = new SlicePool(VIEW_POOL_SIZE); + return viewPool.acquire(abv.array(), abv.offset() + s, l); + } + final int fs = s, fl = l; return new ByteView() { - public int length() { return l; } - public byte byteAt(int idx) { return source.byteAt(s + idx); } + public int length() { return fl; } + public byte byteAt(int idx) { return source.byteAt(fs + idx); } }; } private int indexOf(String name) { - for (int i = 0; i < names.length; i++) if (names[i].equals(name)) return i; + for (int i = 0; i < count; i++) if (names[i].equals(name)) return i; return -1; } } diff --git a/flash/src/main/java/dev/relism/flash/models/QueryParams.java b/flash/src/main/java/dev/relism/flash/models/QueryParams.java index d9c6a9f..45c3aaf 100644 --- a/flash/src/main/java/dev/relism/flash/models/QueryParams.java +++ b/flash/src/main/java/dev/relism/flash/models/QueryParams.java @@ -1,5 +1,8 @@ package dev.relism.flash.models; +import dev.relism.flash.bytes.ArrayBackedByteView; +import dev.relism.flash.bytes.Pairs; +import dev.relism.flash.bytes.SlicePool; import dev.relism.fpr.core.ByteView; import java.nio.charset.StandardCharsets; @@ -15,9 +18,16 @@ import java.util.List; */ public class QueryParams { public static final QueryParams EMPTY = new QueryParams(null); + private static final int VIEW_POOL_SIZE = 4; private final ByteView raw; + // EX-05: created lazily, only if view() is ever actually called — QueryParams itself is + // recreated per request (see Request#resolveQueryParams), so an eagerly-constructed pool + // would cost VIEW_POOL_SIZE allocations on every request that touches query params at all, + // even the (currently: every) request that never calls view(). + private SlicePool viewPool; + public QueryParams(ByteView raw) { this.raw = raw; } @@ -25,16 +35,30 @@ public class QueryParams { public String get(String name) { long r = findFirst(name); if (r < 0) return null; - return decode((int) (r >> 32), (int) (r & 0xFFFFFFFFL)); + return decode(Pairs.hi(r), Pairs.lo(r)); } + /** + * Returns a view over the first raw (not percent-decoded) value of {@code name}, or + * {@code null}. {@code EX-05}: drawn from a small internal {@link SlicePool} when + * {@link #raw} is array-backed (always true for h1 today) instead of allocated per call — + * same reuse-window contract as {@link HeaderMap#view}: valid until either the request ends + * or {@link #view} is called {@value #VIEW_POOL_SIZE} more times on this instance, whichever + * comes first. Falls back to a fresh (allocating) view when {@link #raw} is not array-backed + * — never exercised on the real request path (see {@link ArrayBackedByteView}'s Javadoc). + */ ByteView view(String name) { long r = findFirst(name); if (r < 0) return null; - final int s = (int) (r >> 32), l = (int) (r & 0xFFFFFFFFL); + int s = Pairs.hi(r), l = Pairs.lo(r); + if (raw instanceof ArrayBackedByteView abv) { + if (viewPool == null) viewPool = new SlicePool(VIEW_POOL_SIZE); + return viewPool.acquire(abv.array(), abv.offset() + s, l); + } + final int fs = s, fl = l; return new ByteView() { - public int length() { return l; } - public byte byteAt(int idx) { return raw.byteAt(s + idx); } + public int length() { return fl; } + public byte byteAt(int idx) { return raw.byteAt(fs + idx); } }; } @@ -62,7 +86,7 @@ public class QueryParams { // ── Internals ───────────────────────────────────────────────────────────── - /** Returns (valStart << 32) | valLen, or -1 if not found. */ + /** Returns {@code (valStart << 32) | valLen}, or {@code -1} if not found. */ private long findFirst(String name) { if (raw == null) return -1L; int i = 0, len = raw.length(); @@ -74,7 +98,7 @@ public class QueryParams { i++; int valStart = i; while (i < len && raw.byteAt(i) != '&') i++; - if (keyMatches(keyStart, keyLen, name)) return ((long) valStart << 32) | (i - valStart); + if (keyMatches(keyStart, keyLen, name)) return Pairs.pack(valStart, i - valStart); } if (i < len && raw.byteAt(i) == '&') i++; } @@ -91,8 +115,27 @@ public class QueryParams { * Percent-decodes a value slice from {@code raw} into a UTF-8 String. * {@code %XX} triplets are decoded to their byte values; {@code +} decodes as space. * Invalid {@code %} sequences are passed through as-is. + * + *

    {@code EX-26}: the overwhelmingly common query value contains neither {@code %} nor + * {@code +} — scanned for first; when clean and {@link #raw} is array-backed, the + * {@code String} is built directly from the backing array in one allocation, skipping the + * scratch {@code byte[]} copy this method used to make unconditionally for every value. */ private String decode(int start, int length) { + boolean clean = true; + for (int i = 0; i < length; i++) { + byte b = raw.byteAt(start + i); + if (b == '%' || b == '+') { clean = false; break; } + } + if (clean) { + if (raw instanceof ArrayBackedByteView abv) { + return new String(abv.array(), abv.offset() + start, length, StandardCharsets.UTF_8); + } + byte[] out = new byte[length]; + for (int i = 0; i < length; i++) out[i] = raw.byteAt(start + i); + return new String(out, StandardCharsets.UTF_8); + } + byte[] out = new byte[length]; // upper bound — decoded is never longer int w = 0; for (int i = 0; i < length; i++) { diff --git a/flash/src/main/java/dev/relism/flash/models/Request.java b/flash/src/main/java/dev/relism/flash/models/Request.java index 2d80b72..3aaa58e 100644 --- a/flash/src/main/java/dev/relism/flash/models/Request.java +++ b/flash/src/main/java/dev/relism/flash/models/Request.java @@ -1,6 +1,7 @@ package dev.relism.flash.models; import dev.relism.flash.RequestParser; +import dev.relism.flash.bytes.ArrayBackedByteView; import dev.relism.fpr.core.ByteView; import dev.relism.flash.http.HttpMethod; import lombok.EqualsAndHashCode; @@ -123,6 +124,12 @@ public class Request { public String path() { if (cachedPath != null) return cachedPath; ByteView v = requestLine.getPath(); + // EX-25: one allocation via a direct String(array, offset, length) construction when the + // view is a contiguous array slice (always true for h1 today), instead of a byte-at-a-time + // copy into a scratch array followed by a second allocation for the String itself. + if (v instanceof ArrayBackedByteView abv) { + return cachedPath = new String(abv.array(), abv.offset(), v.length(), StandardCharsets.UTF_8); + } byte[] buf = new byte[v.length()]; for (int i = 0; i < v.length(); i++) buf[i] = v.byteAt(i); return cachedPath = new String(buf, StandardCharsets.UTF_8); diff --git a/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java b/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java index 1ccd6d9..58c9baa 100644 --- a/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java +++ b/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java @@ -6,7 +6,6 @@ import dev.relism.flash.routing.routers.fastpathrouter.FastPathRouterImpl; import dev.relism.flash.Flash; import dev.relism.flash.http.ContentType; import dev.relism.flash.http.HttpMethod; -import dev.relism.fpr.core.ByteView; import dev.relism.flash.template.ErrorPages; import java.nio.charset.StandardCharsets; @@ -101,14 +100,33 @@ public abstract class AbstractRouter { // ── Routing ────────────────────────────────────────────────────────────── - public abstract RequestHandler route(Request request); + /** + * Creates a fresh per-connection scratch object for {@link #route}, or {@code null} if this + * router implementation keeps no reusable per-connection state. Called once per connection + * by the connection driver (e.g. {@code Http1Connection}), which holds the opaque result and + * passes it back into every {@link #route} call for that connection's whole lifetime — the + * same "create once per connection, reuse across requests" shape already used there for + * {@code RequestParser}. + * + *

    {@code EX-06}'s router-half fix: a {@code ThreadLocal} here would mean "one per virtual + * thread", which under this codebase's one-virtual-thread-per-connection model is "one per + * connection with no upper bound and no pooling" — exactly the failure mode + * {@code ConnectionScratch} already exists to avoid for every other per-connection buffer. + * An explicit, caller-owned scratch object achieves the same per-connection reuse without + * that unbounded-growth risk, and without requiring {@code routing} to depend on + * {@code transport}'s {@code ConnectionScratch} type (this package has no such dependency + * today — see {@code DECISIONS.md}, {@code DEC-19}, for why that boundary was kept rather + * than extending {@code ConnectionScratch} itself, which is what an earlier draft of this + * fix assumed). + */ + public Object newScratch() { + return null; + } + + public abstract RequestHandler route(Request request, Object scratch); protected abstract AbstractRouter addRoute(HttpMethod method, String path, RequestHandler handler); - protected static void setPathParams(Request request, String[] names, ByteView source, int[] starts, int[] lens) { - PathParams.inject(request, new PathParams(source, names, starts, lens)); - } - @FunctionalInterface public interface ExceptionHandler { Object handle(Exception exception, Request request, Response response); diff --git a/flash/src/main/java/dev/relism/flash/routing/AbstractWsRouter.java b/flash/src/main/java/dev/relism/flash/routing/AbstractWsRouter.java index 6b78755..86252bf 100644 --- a/flash/src/main/java/dev/relism/flash/routing/AbstractWsRouter.java +++ b/flash/src/main/java/dev/relism/flash/routing/AbstractWsRouter.java @@ -15,7 +15,17 @@ public abstract class AbstractWsRouter { return addRoute(method, PathUtils.sanitize(path), handler); } - public abstract WebSocketHandler route(Request request); + /** + * Creates a fresh per-connection scratch object for {@link #route}, or {@code null} if this + * router keeps no reusable per-connection state — see {@link AbstractRouter#newScratch} for + * the full rationale ({@code EX-06}'s router-half fix), mirrored here for the WebSocket + * router. + */ + public Object newScratch() { + return null; + } + + public abstract WebSocketHandler route(Request request, Object scratch); protected abstract AbstractWsRouter addRoute(HttpMethod method, String path, WebSocketHandler handler); diff --git a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImpl.java b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImpl.java index 6041521..fa712f6 100644 --- a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImpl.java +++ b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImpl.java @@ -6,15 +6,21 @@ import dev.relism.fpr.core.MatchResult; import dev.relism.fpr.core.RouterBuilder; import dev.relism.fpr.core.dsl.StringRouteParser; import dev.relism.flash.http.HttpMethod; +import dev.relism.flash.models.PathParams; import dev.relism.flash.models.Request; import dev.relism.flash.models.RequestHandler; import dev.relism.flash.routing.AbstractRouter; +import java.util.Arrays; + /** * Router backed by the {@code fpr-core} byte-level state machine. Routes are compiled lazily * on the first request and recompiled when routes are added after startup. Matching runs on a - * virtual {@code METHOD + path} byte sequence in a single pass; {@link MatchResult} and - * {@link FastPathViews.MethodPathByteView} are reused per-thread to avoid hot-path allocations. + * virtual {@code METHOD + path} byte sequence in a single pass; the per-connection + * {@link RouteScratch} ({@link #newScratch}) owns the reused {@link MatchResult}, + * {@link FastPathViews.MethodPathByteView} and path-param arrays that would otherwise allocate + * (or, before {@code EX-06}'s router-half fix, sit in an unbounded {@code ThreadLocal}) on every + * request. */ public class FastPathRouterImpl extends AbstractRouter { private final RouterBuilder builder = new RouterBuilder<>(); @@ -23,21 +29,49 @@ public class FastPathRouterImpl extends AbstractRouter { public FastPathRouterImpl() {} - private static final class FastPathRouterContext { - private static final ThreadLocal> RESULT_HOLDER = - ThreadLocal.withInitial(() -> new MatchResult<>(32, 128)); - private static final ThreadLocal COMBINED_VIEW_HOLDER = - ThreadLocal.withInitial(FastPathViews.MethodPathByteView::new); + /** + * Per-connection reusable matching state — {@code EX-06}'s router half and {@code EX-19} + * together. Created once per connection by {@link #newScratch} and threaded back into every + * {@link #route} call for that connection's lifetime (see {@link AbstractRouter#newScratch} + * for why this replaced the two {@code ThreadLocal}s this class used to hold). + * + *

    {@code paramNames}/{@code paramStarts}/{@code paramLens} ({@code EX-19}) start small and + * grow (doubling, via {@link #ensureParamCapacity}) to the connection's high-water mark — + * the number of path params the most param-heavy route matched on this connection ever + * needed — and are never shrunk back down or reallocated once warm, the same amortized policy + * {@code RequestParser}'s read buffer already uses. {@code pathParams} is the single + * {@link PathParams} instance repositioned (via {@link PathParams#reset}) over those arrays + * every time a match has params, instead of a fresh {@code PathParams} per request. + */ + static final class RouteScratch { + final MatchResult matchResult = new MatchResult<>(32, 128); + final FastPathViews.MethodPathByteView combinedView = new FastPathViews.MethodPathByteView(); - public static MatchResult getResult() { - return RESULT_HOLDER.get(); - } + String[] paramNames = new String[8]; + int[] paramStarts = new int[8]; + int[] paramLens = new int[8]; + PathParams pathParams = new PathParams(paramNames, paramStarts, paramLens); - public static FastPathViews.MethodPathByteView getCombinedView() { - return COMBINED_VIEW_HOLDER.get(); + void ensureParamCapacity(int count) { + if (count <= paramNames.length) return; + int grown = paramNames.length; + while (grown < count) grown *= 2; + paramNames = Arrays.copyOf(paramNames, grown); + paramStarts = Arrays.copyOf(paramStarts, grown); + paramLens = Arrays.copyOf(paramLens, grown); + // The arrays PathParams reads are now different instances — rebuild it. This is the + // only case in which a RouteScratch allocates past connection setup, and only on a + // connection whose route mix keeps needing more params than ever seen before; it + // never happens again once this connection's high-water mark stabilizes. + pathParams = new PathParams(paramNames, paramStarts, paramLens); } } + @Override + public Object newScratch() { + return new RouteScratch(); + } + @Override protected AbstractRouter addRoute(HttpMethod method, String path, RequestHandler handler) { builder.add(StringRouteParser.parse(method.name() + path), handler); @@ -46,15 +80,16 @@ public class FastPathRouterImpl extends AbstractRouter { } @Override - public RequestHandler route(Request request) { + public RequestHandler route(Request request, Object scratchObj) { ensureCompiled(); + RouteScratch scratch = (RouteScratch) scratchObj; - MatchResult result = FastPathRouterContext.getResult(); + MatchResult result = scratch.matchResult; result.reset(); HttpMethod method = request.getRequestLine().getMethod(); ByteView pathView = request.getRequestLine().getPath(); - FastPathViews.MethodPathByteView combinedView = FastPathRouterContext.getCombinedView(); + FastPathViews.MethodPathByteView combinedView = scratch.combinedView; combinedView.reset(method.getBytes(), pathView); int labelId = router.match(combinedView, result); @@ -65,18 +100,20 @@ public class FastPathRouterImpl extends AbstractRouter { int count = result.paramCount(); if (count > 0) { - int methodLen = method.getBytes().length; - String[] all = cachedParamNames; - String[] names = new String[count]; - int[] starts = new int[count]; - int[] lens = new int[count]; + scratch.ensureParamCapacity(count); + int methodLen = method.getBytes().length; + String[] all = cachedParamNames; + String[] names = scratch.paramNames; + int[] starts = scratch.paramStarts; + int[] lens = scratch.paramLens; for (int i = 0; i < count; i++) { names[i] = all[result.keyIdAt(i)]; starts[i] = result.startAt(i) - methodLen; lens[i] = result.lenAt(i); } - setPathParams(request, names, pathView, starts, lens); + scratch.pathParams.reset(pathView, count); + PathParams.inject(request, scratch.pathParams); } return result.handler(); diff --git a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java index 2473c87..b2a2abd 100644 --- a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java +++ b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViews.java @@ -1,15 +1,47 @@ package dev.relism.flash.routing.routers.fastpathrouter; +import dev.relism.flash.bytes.ArrayBackedByteView; import dev.relism.fpr.core.ByteView; import lombok.NoArgsConstructor; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; /** {@link dev.relism.fpr.core.ByteView} implementations used on the router and parser hot paths. */ @NoArgsConstructor public final class FastPathViews { - public static final class RequestByteView implements ByteView { + /** + * {@code EX-04}: {@code fpr-core}'s decompiled {@code ByteCompare} (its word-at-a-time + * router-matching fast path — see {@code ByteCompare.equals}/{@code indexOf}) reads a + * comparison word via {@code MethodHandles.byteArrayViewVarHandle(long[].class, + * ByteOrder.LITTLE_ENDIAN)} and compares it bit-for-bit against whatever + * {@link ByteView#longAt} returns. For that comparison to be correct, {@code longAt} must + * therefore return the identical little-endian-assembled value for the same 8 + * bytes — fixed to {@code LITTLE_ENDIAN} specifically (not {@code nativeOrder()}) so the + * contract holds on every host regardless of the JVM's native byte order, matching + * {@code ByteCompare}'s own fixed choice exactly. Confirmed by decompiling + * {@code fpr-core-1.1.1}'s {@code ByteCompare.class} (its {@code LONG_VIEW} field), not + * merely assumed — see {@code FastPathViewsLongAtTest} for the runtime verification the + * plan requires beyond reading bytecode. + */ + private static final VarHandle LONG_VIEW_LE = + MethodHandles.byteArrayViewVarHandle(long[].class, ByteOrder.LITTLE_ENDIAN); + + /** + * Reads 8 bytes at {@code array[pos, pos + 8)} as fpr-core's {@code ByteCompare} expects a + * {@link ByteView#longAt} implementation to. Caller-guaranteed contract (never asserted here + * — {@code ByteCompare} itself never calls this without first checking {@code pos + 8 <= + * length}, so a defensive check here would be dead code on every real call path; see + * {@code EX-04}'s registry entry): {@code pos + 8 <= array.length}. + */ + private static long longAtLittleEndian(byte[] array, int pos) { + return (long) LONG_VIEW_LE.get(array, pos); + } + + public static final class RequestByteView implements ArrayBackedByteView { private final byte[] buffer; private final int start; private final int length; @@ -33,13 +65,49 @@ public final class FastPathViews { return buffer[start + index]; } + @Override + public byte[] array() { + return buffer; + } + + @Override + public int offset() { + return start; + } + + /** {@code EX-04}: array-backed and contiguous — the word-at-a-time router path applies. */ + @Override + public boolean supportsLong() { + return true; + } + + @Override + public long longAt(int index) { + return longAtLittleEndian(buffer, start + index); + } + @Override public String toString() { return new String(buffer, start, length, StandardCharsets.UTF_8); } } - /** Mutable composite view: method bytes + path. Reused via ThreadLocal, call reset() before use. */ + /** + * Mutable composite view: method bytes + path. Reused per connection, call {@link #reset} + * before use (see {@code FastPathRouterImpl}'s per-connection scratch, {@code EX-06}). + * + *

    {@code EX-04}: deliberately not array-backed, {@code supportsLong()} stays {@code false}

    + * Unlike every other view in this file, this one is a composite of two independent sources + * (a raw {@code byte[]} for the method, and another {@link ByteView} — itself possibly + * array-backed — for the path). There is no single backing array a word-at-a-time read could + * span, and a byte index near the method/path boundary could straddle both sources entirely, + * making a single contiguous 8-byte read structurally impossible in general (not merely + * unimplemented) — the same reasoning {@link dev.relism.flash.bytes.SegmentedByteView} + * documents for the analogous HPACK CONTINUATION case. Falls back to the inherited + * {@link ByteView#supportsLong} default ({@code false}); {@code fpr-core}'s router-matching + * path already handles that correctly (it only takes the word-at-a-time branch when + * {@code supportsLong()} is {@code true}). + */ public static final class MethodPathByteView implements ByteView { private byte[] method; private ByteView path; @@ -62,7 +130,7 @@ public final class FastPathViews { } } - public static class SocketByteView implements ByteView { + public static class SocketByteView implements ArrayBackedByteView { private final byte[] data; public SocketByteView(byte[] data) { @@ -78,9 +146,30 @@ public final class FastPathViews { public byte byteAt(int index) { return data[index]; } + + @Override + public byte[] array() { + return data; + } + + @Override + public int offset() { + return 0; + } + + /** {@code EX-04}: array-backed and contiguous — the word-at-a-time router path applies. */ + @Override + public boolean supportsLong() { + return true; + } + + @Override + public long longAt(int index) { + return longAtLittleEndian(data, index); + } } - public static class StringByteView implements ByteView { + public static class StringByteView implements ArrayBackedByteView { private final byte[] bytes; public StringByteView(String str) { @@ -96,5 +185,26 @@ public final class FastPathViews { public byte byteAt(int index) { return bytes[index]; } + + @Override + public byte[] array() { + return bytes; + } + + @Override + public int offset() { + return 0; + } + + /** {@code EX-04}: array-backed and contiguous — the word-at-a-time router path applies. */ + @Override + public boolean supportsLong() { + return true; + } + + @Override + public long longAt(int index) { + return longAtLittleEndian(bytes, index); + } } } diff --git a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathWsRouterImpl.java b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathWsRouterImpl.java index 339fe4d..ebd3f60 100644 --- a/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathWsRouterImpl.java +++ b/flash/src/main/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathWsRouterImpl.java @@ -10,12 +10,35 @@ import dev.relism.flash.models.Request; import dev.relism.flash.routing.AbstractWsRouter; import dev.relism.flash.websocket.WebSocketHandler; +/** + * WebSocket-upgrade counterpart of {@link FastPathRouterImpl} — same {@code fpr-core} matching + * engine, same {@code EX-06} router-half fix (an explicit per-connection {@link RouteScratch} + * via {@link #newScratch} in place of the {@code ThreadLocal}s this class used to hold). Unlike + * {@link FastPathRouterImpl}, its path-param extraction is not covered by {@code EX-19} (that + * registry entry names {@code FastPathRouterImpl.route} specifically) and still allocates a + * fresh {@code PathParams} per matched, parametric WebSocket upgrade — WebSocket upgrades are + * inherently rare relative to ordinary requests (one per connection, not one per message), so + * this was not flagged as a hot-path allocation concern. + */ public final class FastPathWsRouterImpl extends AbstractWsRouter { private final RouterBuilder builder = new RouterBuilder<>(); private volatile FastPathRouter router; private String[] cachedParamNames; + /** Per-connection reusable matching state — see {@link FastPathRouterImpl.RouteScratch}'s + * javadoc for the full {@code EX-06} rationale; this router's scratch is smaller since + * {@code EX-19}'s path-param reuse does not apply here (see the class Javadoc). */ + static final class RouteScratch { + final MatchResult matchResult = new MatchResult<>(32, 128); + final FastPathViews.MethodPathByteView combinedView = new FastPathViews.MethodPathByteView(); + } + + @Override + public Object newScratch() { + return new RouteScratch(); + } + @Override protected AbstractWsRouter addRoute(HttpMethod method, String path, WebSocketHandler handler) { builder.add(StringRouteParser.parse(method.name() + path), handler); @@ -24,15 +47,16 @@ public final class FastPathWsRouterImpl extends AbstractWsRouter { } @Override - public WebSocketHandler route(Request request) { + public WebSocketHandler route(Request request, Object scratchObj) { ensureCompiled(); + RouteScratch scratch = (RouteScratch) scratchObj; - MatchResult result = Context.result(); + MatchResult result = scratch.matchResult; result.reset(); HttpMethod method = request.getRequestLine().getMethod(); ByteView pathView = request.getRequestLine().getPath(); - FastPathViews.MethodPathByteView combined = Context.combined(); + FastPathViews.MethodPathByteView combined = scratch.combinedView; combined.reset(method.getBytes(), pathView); int labelId = router.match(combined, result); @@ -58,14 +82,4 @@ public final class FastPathWsRouterImpl extends AbstractWsRouter { @Override public void compile() { ensureCompiled(); } - - private static final class Context { - private static final ThreadLocal> RESULT = - ThreadLocal.withInitial(() -> new MatchResult<>(32, 128)); - private static final ThreadLocal COMBINED = - ThreadLocal.withInitial(FastPathViews.MethodPathByteView::new); - - static MatchResult result() { return RESULT.get(); } - static FastPathViews.MethodPathByteView combined() { return COMBINED.get(); } - } } diff --git a/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java b/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java index 4866d07..61c4fe3 100644 --- a/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java +++ b/flash/src/main/java/dev/relism/flash/transport/ConnectionScratch.java @@ -23,10 +23,13 @@ import java.security.NoSuchAlgorithmException; * pool when the connection closes. Never shared between two connections at once — there is no * synchronization here because none is needed. * - *

    Extended in Phase 4 with the router's reusable {@code MatchResult}/path-view fields - * (currently still {@code ThreadLocal} in {@code FastPathRouterImpl}, per {@code EX-06}'s own - * multi-phase assignment — see {@code DECISIONS.md} for why Phase 2 does not also absorb that - * part of the fix) and in later phases with HTTP/2 write/HPACK scratch. + *

    {@code EX-06}'s router half (the {@code FastPathRouterImpl}/{@code FastPathWsRouterImpl} + * {@code ThreadLocal}s) is fixed in Phase 4, but deliberately not by extending this + * class: {@code routing} has no dependency on {@code transport} today, and folding the router's + * scratch fields in here would have created one — see {@code DECISIONS.md}, {@code DEC-19}, for + * the opaque-per-connection-object mechanism ({@code AbstractRouter#newScratch}) used instead. + * This class gains HTTP/2 write/HPACK scratch in later phases, where {@code h2} already depends + * on {@code transport} and no such boundary concern applies. */ public final class ConnectionScratch { diff --git a/flash/src/test/java/dev/relism/flash/bytes/ByteScanFuzzTest.java b/flash/src/test/java/dev/relism/flash/bytes/ByteScanFuzzTest.java new file mode 100644 index 0000000..b28626b --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/bytes/ByteScanFuzzTest.java @@ -0,0 +1,67 @@ +package dev.relism.flash.bytes; + +import org.junit.jupiter.api.Test; + +import java.util.Random; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +/** + * Randomized agreement testing for {@link ByteScan}'s SWAR methods against their scalar + * counterparts, per Phase 4's task 1 ("property-test SWAR against scalar on random inputs of + * every length 0..256 ... including unaligned starts"). {@link ByteScanTest} already covers + * every exact boundary deterministically; this class instead throws a large volume of fully + * random bytes and random sub-ranges at both implementations, on a fixed seed for reproducible + * CI failures, purely to catch any interaction between random byte content and the SWAR bit + * tricks that a hand-picked boundary test would miss. + */ +class ByteScanFuzzTest { + + private static final int TRIALS = 20_000; + private static final int MAX_LEN = 300; + + @Test + void indexOf_agreesWithScalar_onFullyRandomInputs() { + Random rnd = new Random(1234567); + for (int t = 0; t < TRIALS; t++) { + int len = rnd.nextInt(MAX_LEN + 1); + byte[] buf = new byte[len]; + rnd.nextBytes(buf); + byte target = (byte) rnd.nextInt(256); + int from = len == 0 ? 0 : rnd.nextInt(len + 1); + int to = from == len ? len : from + rnd.nextInt(len - from + 1); + + int expected = ByteScan.indexOfScalar(buf, from, to, target); + int actual = assertDoesNotThrow(() -> ByteScan.indexOf(buf, from, to, target)); + int trial = t; + assertEquals(expected, actual, + () -> "mismatch at trial " + trial + ", len=" + len + ", from=" + from + ", to=" + to); + } + } + + @Test + void indexOfCrLfCrLf_agreesWithScalar_onFullyRandomInputs() { + Random rnd = new Random(9876543); + for (int t = 0; t < TRIALS; t++) { + int len = rnd.nextInt(MAX_LEN + 1); + byte[] buf = new byte[len]; + rnd.nextBytes(buf); + // Occasionally bias toward CR/LF bytes so real matches (and near-matches) show up, + // not just "no CR anywhere" cases. + if (rnd.nextInt(3) == 0) { + for (int i = 0; i < len; i++) { + if (rnd.nextInt(4) == 0) buf[i] = rnd.nextBoolean() ? (byte) '\r' : (byte) '\n'; + } + } + int from = len == 0 ? 0 : rnd.nextInt(len + 1); + int to = from == len ? len : from + rnd.nextInt(len - from + 1); + + int expected = ByteScan.indexOfCrLfCrLfScalar(buf, from, to); + int actual = assertDoesNotThrow(() -> ByteScan.indexOfCrLfCrLf(buf, from, to)); + int trial = t; + assertEquals(expected, actual, + () -> "mismatch at trial " + trial + ", len=" + len + ", from=" + from + ", to=" + to); + } + } +} diff --git a/flash/src/test/java/dev/relism/flash/bytes/ByteScanTest.java b/flash/src/test/java/dev/relism/flash/bytes/ByteScanTest.java new file mode 100644 index 0000000..17bf9f1 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/bytes/ByteScanTest.java @@ -0,0 +1,247 @@ +package dev.relism.flash.bytes; + +import dev.relism.fpr.core.ByteView; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; +import java.util.Random; + +import static org.junit.jupiter.api.Assertions.*; + +class ByteScanTest { + + private static final class ArrayView implements ByteView { + final byte[] buf; + ArrayView(String s) { this.buf = s.getBytes(StandardCharsets.US_ASCII); } + @Override public int length() { return buf.length; } + @Override public byte byteAt(int i) { return buf[i]; } + } + + // ── isTChar ────────────────────────────────────────────────────────────── + + @Test + void isTChar_acceptsRfc9110TCharSet() { + for (char c = '0'; c <= '9'; c++) assertTrue(ByteScan.isTChar((byte) c)); + for (char c = 'A'; c <= 'Z'; c++) assertTrue(ByteScan.isTChar((byte) c)); + for (char c = 'a'; c <= 'z'; c++) assertTrue(ByteScan.isTChar((byte) c)); + for (byte b : "!#$%&'*+-.^_`|~".getBytes(StandardCharsets.US_ASCII)) assertTrue(ByteScan.isTChar(b)); + } + + @Test + void isTChar_rejectsDelimitersAndControlAndHighBytes() { + for (byte b : " \t\":;,()<>[]{}=?@\\/".getBytes(StandardCharsets.US_ASCII)) { + assertFalse(ByteScan.isTChar(b), "byte '" + (char) b + "' must not be a tchar"); + } + assertFalse(ByteScan.isTChar((byte) 0)); + assertFalse(ByteScan.isTChar((byte) 127)); + assertFalse(ByteScan.isTChar((byte) -1)); // high-bit byte, e.g. UTF-8 continuation + } + + // ── indexOf: SWAR vs scalar, every boundary ───────────────────────────── + + @Test + void indexOf_swarAgreesWithScalar_everyLengthAndPosition() { + Random rnd = new Random(42); + for (int len = 0; len <= 256; len++) { + byte[] buf = new byte[len]; + rnd.nextBytes(buf); + // Ensure the target byte value (7) doesn't appear anywhere except where we plant it. + for (int i = 0; i < len; i++) if (buf[i] == 7) buf[i] = 8; + + assertEquals(-1, ByteScan.indexOfScalar(buf, 0, len, (byte) 7)); + assertEquals(ByteScan.indexOfScalar(buf, 0, len, (byte) 7), ByteScan.indexOf(buf, 0, len, (byte) 7)); + + for (int pos = 0; pos < len; pos++) { + byte[] planted = buf.clone(); + planted[pos] = 7; + int expected = ByteScan.indexOfScalar(planted, 0, len, (byte) 7); + assertEquals(pos, expected, "scalar oracle disagrees with itself at pos " + pos); + assertEquals(expected, ByteScan.indexOf(planted, 0, len, (byte) 7), + "SWAR disagrees with scalar at len=" + len + " pos=" + pos); + } + } + } + + @Test + void indexOf_unalignedStart_agreesWithScalar() { + Random rnd = new Random(7); + byte[] buf = new byte[64]; + rnd.nextBytes(buf); + for (int i = 0; i < buf.length; i++) if (buf[i] == 9) buf[i] = 10; + buf[40] = 9; + for (int from = 0; from < 8; from++) { + assertEquals(ByteScan.indexOfScalar(buf, from, buf.length, (byte) 9), + ByteScan.indexOf(buf, from, buf.length, (byte) 9)); + } + } + + // ── indexOfCrLfCrLf: SWAR vs scalar, every boundary ───────────────────── + + @Test + void indexOfCrLfCrLf_swarAgreesWithScalar_everyLengthAndPosition() { + Random rnd = new Random(99); + for (int len = 4; len <= 128; len++) { + byte[] base = new byte[len]; + rnd.nextBytes(base); + // Strip any accidental \r or \n so only the planted match exists. + for (int i = 0; i < len; i++) { + if (base[i] == '\r' || base[i] == '\n') base[i] = 'x'; + } + assertEquals(-1, ByteScan.indexOfCrLfCrLfScalar(base, 0, len)); + assertEquals(-1, ByteScan.indexOfCrLfCrLf(base, 0, len)); + + for (int pos = 0; pos <= len - 4; pos++) { + byte[] planted = base.clone(); + planted[pos] = '\r'; planted[pos + 1] = '\n'; planted[pos + 2] = '\r'; planted[pos + 3] = '\n'; + int expected = ByteScan.indexOfCrLfCrLfScalar(planted, 0, len); + assertEquals(pos, expected); + assertEquals(expected, ByteScan.indexOfCrLfCrLf(planted, 0, len), + "SWAR disagrees with scalar at len=" + len + " pos=" + pos); + } + } + } + + @Test + void indexOfCrLfCrLf_matchAtVeryLastPossiblePosition() { + byte[] buf = "GET / HTTP/1.1\r\n\r\n".getBytes(StandardCharsets.US_ASCII); + int expected = buf.length - 4; + assertEquals(expected, ByteScan.indexOfCrLfCrLf(buf, 0, buf.length)); + } + + @Test + void indexOfCrLfCrLf_bareCrNotFollowedByLf_isNotAMatch() { + byte[] buf = "a\r\rb\r\n\r\nc".getBytes(StandardCharsets.US_ASCII); + int expected = ByteScan.indexOfCrLfCrLfScalar(buf, 0, buf.length); + assertEquals(expected, ByteScan.indexOfCrLfCrLf(buf, 0, buf.length)); + assertTrue(expected >= 0); + } + + @Test + void indexOfCrLfCrLf_lengthNotMultipleOfEight_doesNotOverrun() { + for (int len = 4; len <= 20; len++) { + byte[] buf = new byte[len]; + for (int i = 0; i < len; i++) buf[i] = 'x'; + assertEquals(-1, ByteScan.indexOfCrLfCrLf(buf, 0, len)); + if (len >= 4) { + buf[len - 4] = '\r'; buf[len - 3] = '\n'; buf[len - 2] = '\r'; buf[len - 1] = '\n'; + assertEquals(len - 4, ByteScan.indexOfCrLfCrLf(buf, 0, len)); + } + } + } + + // ── Case-insensitive comparison ───────────────────────────────────────── + + @Test + void equalsIgnoreCaseAscii_array_matchesRegardlessOfCase() { + byte[] buf = "Content-Type".getBytes(StandardCharsets.US_ASCII); + assertTrue(ByteScan.equalsIgnoreCaseAscii(buf, 0, buf.length, "content-type")); + assertTrue(ByteScan.equalsIgnoreCaseAscii(buf, 0, buf.length, "CONTENT-TYPE")); + assertFalse(ByteScan.equalsIgnoreCaseAscii(buf, 0, buf.length, "content-length")); + } + + @Test + void equalsIgnoreCaseAscii_twoArrays() { + byte[] a = "Accept".getBytes(StandardCharsets.US_ASCII); + byte[] b = "aCCEPT".getBytes(StandardCharsets.US_ASCII); + assertTrue(ByteScan.equalsIgnoreCaseAscii(a, 0, a.length, b, 0, b.length)); + byte[] c = "Accept-X".getBytes(StandardCharsets.US_ASCII); + assertFalse(ByteScan.equalsIgnoreCaseAscii(a, 0, a.length, c, 0, c.length)); + } + + @Test + void equalsIgnoreCase_view() { + ArrayView v = new ArrayView("Keep-Alive"); + assertTrue(ByteScan.equalsIgnoreCase(v, 0, v.length(), "keep-alive")); + assertFalse(ByteScan.equalsIgnoreCase(v, 0, v.length(), "close")); + } + + // ── Token lists ────────────────────────────────────────────────────────── + + @Test + void tokenListContains_findsTokenAmongMultiple() { + ArrayView v = new ArrayView("keep-alive, Upgrade"); + assertTrue(ByteScan.tokenListContains(v, "upgrade")); + assertTrue(ByteScan.tokenListContains(v, "keep-alive")); + assertFalse(ByteScan.tokenListContains(v, "close")); + } + + @Test + void tokenListContains_singleToken() { + ArrayView v = new ArrayView("close"); + assertTrue(ByteScan.tokenListContains(v, "close")); + } + + @Test + void tokenListContains_emptyList() { + ArrayView v = new ArrayView(""); + assertFalse(ByteScan.tokenListContains(v, "close")); + } + + // ── Header-name hash ───────────────────────────────────────────────────── + + @Test + void hashNameIgnoreCaseAscii_isCaseInsensitive() { + byte[] lower = "content-length".getBytes(StandardCharsets.US_ASCII); + byte[] mixed = "Content-Length".getBytes(StandardCharsets.US_ASCII); + byte[] upper = "CONTENT-LENGTH".getBytes(StandardCharsets.US_ASCII); + int h1 = ByteScan.hashNameIgnoreCaseAscii(lower, 0, lower.length); + int h2 = ByteScan.hashNameIgnoreCaseAscii(mixed, 0, mixed.length); + int h3 = ByteScan.hashNameIgnoreCaseAscii(upper, 0, upper.length); + assertEquals(h1, h2); + assertEquals(h2, h3); + } + + @Test + void hashNameIgnoreCaseAscii_stringOverloadAgreesWithByteArrayOverload() { + for (String name : new String[]{"content-length", "Content-Length", "CONTENT-LENGTH", "x", ""}) { + byte[] b = name.getBytes(StandardCharsets.US_ASCII); + assertEquals(ByteScan.hashNameIgnoreCaseAscii(b, 0, b.length), ByteScan.hashNameIgnoreCaseAscii(name)); + } + } + + @Test + void hashNameIgnoreCaseAscii_differentNamesUsuallyDiffer() { + String[] names = {"content-length", "content-type", "authorization", "cookie", "accept", + "host", "user-agent", "x-forwarded-for", "connection", "upgrade"}; + java.util.Set hashes = new java.util.HashSet<>(); + for (String n : names) { + byte[] b = n.getBytes(StandardCharsets.US_ASCII); + hashes.add(ByteScan.hashNameIgnoreCaseAscii(b, 0, b.length)); + } + assertEquals(names.length, hashes.size(), "expected no collisions among common header names"); + } + + // ── Decimal / hex parsing ──────────────────────────────────────────────── + + @Test + void parseDecimalStrict_validAndInvalidCases() { + assertEquals(0L, parse("0")); + assertEquals(12345L, parse("12345")); + assertEquals(Long.MAX_VALUE, parse(Long.toString(Long.MAX_VALUE))); + assertEquals(ByteScan.PARSE_INVALID, parse("")); + assertEquals(ByteScan.PARSE_INVALID, parse("12a45")); + assertEquals(ByteScan.PARSE_INVALID, parse("-1")); + assertEquals(ByteScan.PARSE_INVALID, parse("+1")); + assertEquals(ByteScan.PARSE_INVALID, parse("99999999999999999999")); // overflow + assertEquals(ByteScan.PARSE_INVALID, parse("10000000000000000000")); // > Long.MAX_VALUE, 20 digits already rejected by length + } + + private static long parse(String s) { + byte[] b = s.getBytes(StandardCharsets.US_ASCII); + return ByteScan.parseDecimalStrict(b, 0, b.length); + } + + @Test + void parseHexStrict_validAndInvalidCases() { + assertEquals(0xFFL, hex("ff", 8)); + assertEquals(0xABCDL, hex("aBcD", 8)); + assertEquals(ByteScan.PARSE_INVALID, hex("", 8)); + assertEquals(ByteScan.PARSE_INVALID, hex("xyz", 8)); + assertEquals(ByteScan.PARSE_INVALID, hex("123456789", 8)); // too many digits + } + + private static long hex(String s, int maxDigits) { + byte[] b = s.getBytes(StandardCharsets.US_ASCII); + return ByteScan.parseHexStrict(b, 0, b.length, maxDigits); + } +} diff --git a/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java b/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java new file mode 100644 index 0000000..222fe80 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/bytes/ByteWriterTest.java @@ -0,0 +1,124 @@ +package dev.relism.flash.bytes; + +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +class ByteWriterTest { + + private static String asString(ByteWriter w) { + return new String(w.array(), 0, w.length(), StandardCharsets.US_ASCII); + } + + @Test + void writeByte_and_writeBytes() { + ByteWriter w = new ByteWriter(4); + w.writeByte((byte) 'H'); + w.writeBytes("ello".getBytes(StandardCharsets.US_ASCII)); + assertEquals("Hello", asString(w)); + } + + @Test + void writeBytes_offsetAndLength() { + ByteWriter w = new ByteWriter(4); + byte[] src = "xxHELLOxx".getBytes(StandardCharsets.US_ASCII); + w.writeBytes(src, 2, 5); + assertEquals("HELLO", asString(w)); + } + + @Test + void growsPastInitialCapacity_withoutLosingData() { + ByteWriter w = new ByteWriter(2); + StringBuilder expected = new StringBuilder(); + for (int i = 0; i < 1000; i++) { + w.writeByte((byte) ('a' + (i % 26))); + expected.append((char) ('a' + (i % 26))); + } + assertEquals(expected.toString(), asString(w)); + } + + @Test + void reset_reusesBufferFromScratch() { + ByteWriter w = new ByteWriter(16); + w.writeBytes("first".getBytes(StandardCharsets.US_ASCII)); + byte[] bufBeforeReset = w.array(); + w.reset(); + assertEquals(0, w.length()); + w.writeBytes("second".getBytes(StandardCharsets.US_ASCII)); + assertEquals("second", asString(w)); + assertSame(bufBeforeReset, w.array(), "reset() must not reallocate when capacity already suffices"); + } + + @Test + void writeDecimal_variousValues() { + assertDecimal("0", 0); + assertDecimal("7", 7); + assertDecimal("42", 42); + assertDecimal("1000000", 1_000_000); + assertDecimal(Long.toString(Long.MAX_VALUE), Long.MAX_VALUE); + } + + private static void assertDecimal(String expected, long value) { + ByteWriter w = new ByteWriter(4); + w.writeDecimal(value); + assertEquals(expected, asString(w)); + } + + @Test + void writeDecimal_rejectsNegative() { + ByteWriter w = new ByteWriter(4); + assertThrows(IllegalArgumentException.class, () -> w.writeDecimal(-1)); + } + + @Test + void writeHex_variousValues() { + assertHex("0", 0); + assertHex("ff", 0xFF); + assertHex("1a2b3c", 0x1A2B3C); + assertHex("ffffffff", 0xFFFFFFFF); + } + + private static void assertHex(String expected, int value) { + ByteWriter w = new ByteWriter(4); + w.writeHex(value); + assertEquals(expected, asString(w)); + } + + @Test + void writeAsciiLower_lowersUppercaseOnly() { + ByteWriter w = new ByteWriter(4); + w.writeAsciiLower("Content-TYPE"); + assertEquals("content-type", asString(w)); + } + + @Test + void writeUInt16_bigEndian() { + ByteWriter w = new ByteWriter(4); + w.writeUInt16(0x1234); + assertArrayEquals(new byte[]{0x12, 0x34}, java.util.Arrays.copyOf(w.array(), w.length())); + } + + @Test + void writeUInt24_bigEndian() { + ByteWriter w = new ByteWriter(4); + w.writeUInt24(0x123456); + assertArrayEquals(new byte[]{0x12, 0x34, 0x56}, java.util.Arrays.copyOf(w.array(), w.length())); + } + + @Test + void writeUInt31_masksTopBit() { + ByteWriter w = new ByteWriter(4); + w.writeUInt31(0xFFFFFFFF); // all bits set -> top bit must be cleared + assertArrayEquals(new byte[]{0x7F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}, + java.util.Arrays.copyOf(w.array(), w.length())); + } + + @Test + void writeUInt32_bigEndian() { + ByteWriter w = new ByteWriter(4); + w.writeUInt32(0x01020304); + assertArrayEquals(new byte[]{0x01, 0x02, 0x03, 0x04}, java.util.Arrays.copyOf(w.array(), w.length())); + } +} diff --git a/flash/src/test/java/dev/relism/flash/bytes/PairsTest.java b/flash/src/test/java/dev/relism/flash/bytes/PairsTest.java new file mode 100644 index 0000000..bfc100e --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/bytes/PairsTest.java @@ -0,0 +1,37 @@ +package dev.relism.flash.bytes; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class PairsTest { + + @Test + void packAndUnpack_roundTrip() { + long p = Pairs.pack(1234, 5678); + assertEquals(1234, Pairs.hi(p)); + assertEquals(5678, Pairs.lo(p)); + } + + @Test + void packAndUnpack_zero() { + long p = Pairs.pack(0, 0); + assertEquals(0, Pairs.hi(p)); + assertEquals(0, Pairs.lo(p)); + } + + @Test + void packAndUnpack_maxInts() { + long p = Pairs.pack(Integer.MAX_VALUE, Integer.MAX_VALUE); + assertEquals(Integer.MAX_VALUE, Pairs.hi(p)); + assertEquals(Integer.MAX_VALUE, Pairs.lo(p)); + } + + @Test + void lo_doesNotSignExtendFromHi() { + // hi negative-looking bit pattern must not bleed into lo after unpack. + long p = Pairs.pack(-1, 42); + assertEquals(-1, Pairs.hi(p)); + assertEquals(42, Pairs.lo(p)); + } +} diff --git a/flash/src/test/java/dev/relism/flash/bytes/SegmentedByteViewTest.java b/flash/src/test/java/dev/relism/flash/bytes/SegmentedByteViewTest.java new file mode 100644 index 0000000..2622e8f --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/bytes/SegmentedByteViewTest.java @@ -0,0 +1,71 @@ +package dev.relism.flash.bytes; + +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +class SegmentedByteViewTest { + + @Test + void reset_presentsSegmentsAsOneLogicalSequence() { + byte[][] segments = { + "Hello, ".getBytes(StandardCharsets.US_ASCII), + "World".getBytes(StandardCharsets.US_ASCII), + "!".getBytes(StandardCharsets.US_ASCII), + }; + int[] offsets = {0, 0, 0}; + int[] lengths = {segments[0].length, segments[1].length, segments[2].length}; + + SegmentedByteView view = new SegmentedByteView(); + view.reset(segments, offsets, lengths, 3); + + assertEquals(13, view.length()); + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < view.length(); i++) sb.append((char) view.byteAt(i)); + assertEquals("Hello, World!", sb.toString()); + } + + @Test + void reset_honorsPerSegmentOffsetAndLength() { + byte[][] segments = { "xxABCxx".getBytes(StandardCharsets.US_ASCII) }; + SegmentedByteView view = new SegmentedByteView(); + view.reset(segments, new int[]{2}, new int[]{3}, 1); + assertEquals(3, view.length()); + assertEquals('A', (char) view.byteAt(0)); + assertEquals('C', (char) view.byteAt(2)); + } + + @Test + void reset_isReusableAcrossCalls() { + SegmentedByteView view = new SegmentedByteView(); + view.reset(new byte[][]{"abc".getBytes(StandardCharsets.US_ASCII)}, new int[]{0}, new int[]{3}, 1); + assertEquals(3, view.length()); + view.reset(new byte[][]{"de".getBytes(StandardCharsets.US_ASCII)}, new int[]{0}, new int[]{2}, 1); + assertEquals(2, view.length()); + assertEquals('d', (char) view.byteAt(0)); + } + + @Test + void byteAt_outOfBoundsThrows() { + SegmentedByteView view = new SegmentedByteView(); + view.reset(new byte[][]{"ab".getBytes(StandardCharsets.US_ASCII)}, new int[]{0}, new int[]{2}, 1); + assertThrows(IndexOutOfBoundsException.class, () -> view.byteAt(2)); + assertThrows(IndexOutOfBoundsException.class, () -> view.byteAt(-1)); + } + + @Test + void supportsLong_alwaysFalse() { + SegmentedByteView view = new SegmentedByteView(); + view.reset(new byte[][]{"12345678".getBytes(StandardCharsets.US_ASCII)}, new int[]{0}, new int[]{8}, 1); + assertFalse(view.supportsLong()); + } + + @Test + void emptySegmentCount_isZeroLength() { + SegmentedByteView view = new SegmentedByteView(); + view.reset(new byte[][]{}, new int[]{}, new int[]{}, 0); + assertEquals(0, view.length()); + } +} diff --git a/flash/src/test/java/dev/relism/flash/bytes/SlicePoolTest.java b/flash/src/test/java/dev/relism/flash/bytes/SlicePoolTest.java new file mode 100644 index 0000000..9333a82 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/bytes/SlicePoolTest.java @@ -0,0 +1,62 @@ +package dev.relism.flash.bytes; + +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +class SlicePoolTest { + + private static byte[] bytes(String s) { return s.getBytes(StandardCharsets.US_ASCII); } + + @Test + void acquire_repositionsAndReturnsRequestedRange() { + SlicePool pool = new SlicePool(4); + byte[] buf = bytes("hello world"); + PooledSlice slice = pool.acquire(buf, 6, 5); + assertEquals(5, slice.length()); + assertEquals('w', (char) slice.byteAt(0)); + assertEquals('d', (char) slice.byteAt(4)); + } + + @Test + void acquire_withinPoolSize_returnsDistinctLiveSlices() { + SlicePool pool = new SlicePool(4); + byte[] buf = bytes("abcdefgh"); + PooledSlice a = pool.acquire(buf, 0, 1); // 'a' + PooledSlice b = pool.acquire(buf, 1, 1); // 'b' + PooledSlice c = pool.acquire(buf, 2, 1); // 'c' + // All three still valid simultaneously — the pool hasn't wrapped yet (size 4). + assertEquals('a', (char) a.byteAt(0)); + assertEquals('b', (char) b.byteAt(0)); + assertEquals('c', (char) c.byteAt(0)); + } + + @Test + void wraparoundAliasesThePreviouslyReturnedSlice() { + // Demonstrates the documented hazard: retaining a slice past `size` further acquire() + // calls observes it silently repositioned to unrelated data. + SlicePool pool = new SlicePool(2); + byte[] buf = bytes("AABB"); + PooledSlice first = pool.acquire(buf, 0, 2); // "AA" + assertEquals('A', (char) first.byteAt(0)); + + pool.acquire(buf, 2, 2); // "BB" — slot 2, pool size 2 so this is still a fresh slot + PooledSlice thirdCall = pool.acquire(buf, 2, 2); // wraps back to `first`'s slot + assertSame(first, thirdCall, "pool of size 2 must reuse the first slot on the 3rd acquire()"); + // `first` is now silently "BB", not "AA" — the documented lifetime contract in action. + assertEquals('B', (char) first.byteAt(0)); + } + + @Test + void constructor_rejectsNonPositiveSize() { + assertThrows(IllegalArgumentException.class, () -> new SlicePool(0)); + assertThrows(IllegalArgumentException.class, () -> new SlicePool(-1)); + } + + @Test + void size_reportsConstructedCapacity() { + assertEquals(4, new SlicePool(4).size()); + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/HeaderMapIndexTest.java b/flash/src/test/java/dev/relism/flash/models/HeaderMapIndexTest.java new file mode 100644 index 0000000..4d24c9d --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/HeaderMapIndexTest.java @@ -0,0 +1,151 @@ +package dev.relism.flash.models; + +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-09}: dedicated correctness coverage for {@link HeaderMap}'s per-{@code reset()} + * index — duplicate names, case variation, zero headers, and growth past the initial index + * capacity up to {@code Http1Limits.MAX_HEADER_COUNT}. {@link HeaderMapTest} already covers the + * ordinary lookup/forEach contract; this class targets the index machinery specifically. + */ +class HeaderMapIndexTest { + + private static HeaderMap parse(String... headers) { + StringBuilder sb = new StringBuilder(); + for (String h : headers) sb.append(h).append("\r\n"); + byte[] buffer = sb.toString().getBytes(StandardCharsets.UTF_8); + HeaderMap map = new HeaderMap(); + map.reset(buffer, 0, buffer.length); + return map; + } + + @Test + void zeroHeaders_everyLookupIsEmpty() { + HeaderMap map = parse(); + assertNull(map.first("Host")); + assertTrue(map.all("Host").isEmpty()); + assertTrue(map.all().isEmpty()); + assertNull(map.view("Host")); + assertFalse(map.valueEqualsIgnoreCase("Connection", "close")); + } + + @Test + void duplicateHeaderNames_firstReturnsTheFirstOne_allReturnsAllInOrder() { + HeaderMap map = parse("X-Trace: a", "X-Trace: b", "X-Trace: c"); + assertEquals("a", map.first("X-Trace")); + assertEquals(List.of("a", "b", "c"), map.all("X-Trace")); + } + + @Test + void caseVariation_indexHashAndCompareBothIgnoreCase() { + HeaderMap map = parse("X-Custom-Header: value1"); + assertEquals("value1", map.first("x-custom-header")); + assertEquals("value1", map.first("X-CUSTOM-HEADER")); + assertEquals("value1", map.first("X-cUsToM-hEaDeR")); + } + + @Test + void similarButDistinctNames_doNotCollideInTheIndex() { + // Names sharing a hash-prefix-adjacent shape must still resolve independently. + HeaderMap map = parse("Accept: a", "Accept-Encoding: b", "Accept-Language: c"); + assertEquals("a", map.first("Accept")); + assertEquals("b", map.first("Accept-Encoding")); + assertEquals("c", map.first("Accept-Language")); + } + + @Test + void growsPastInitialIndexCapacity_upToMaxHeaderCount_andStaysCorrect() { + int n = dev.relism.flash.http.Http1Limits.MAX_HEADER_COUNT; + String[] headers = new String[n]; + for (int i = 0; i < n; i++) headers[i] = "X-Header-" + i + ": value-" + i; + HeaderMap map = parse(headers); + + assertEquals("value-0", map.first("X-Header-0")); + assertEquals("value-" + (n - 1), map.first("X-Header-" + (n - 1))); + assertEquals("value-" + (n / 2), map.first("X-Header-" + (n / 2))); + assertEquals(n, map.all().size()); + } + + @Test + void reset_rebuildsIndexFromScratch_noStaleEntriesFromPreviousRequest() { + HeaderMap map = parse("Host: first-request"); + assertEquals("first-request", map.first("Host")); + assertNull(map.first("X-Only-In-Second")); + + byte[] second = "Host: second-request\r\nX-Only-In-Second: yes\r\n".getBytes(StandardCharsets.UTF_8); + map.reset(second, 0, second.length); + + assertEquals("second-request", map.first("Host")); + assertEquals("yes", map.first("X-Only-In-Second")); + } + + @Test + void repeatedResetsAcrossVaryingHeaderCounts_shrinkAndGrowSafely() { + // A connection whose successive keep-alive requests have very different header counts + // must never see stale entries from a larger previous request bleed into a smaller one. + HeaderMap map = new HeaderMap(); + for (int round = 0; round < 5; round++) { + int n = (round % 2 == 0) ? 20 : 2; + String[] headers = new String[n]; + for (int i = 0; i < n; i++) headers[i] = "H" + i + ": v" + i + "-" + round; + byte[] buf = String.join("\r\n", headers).concat("\r\n").getBytes(StandardCharsets.UTF_8); + map.reset(buf, 0, buf.length); + + assertEquals(n, map.all().size(), "round " + round); + assertEquals("v0-" + round, map.first("H0")); + if (n < 20) assertNull(map.first("H19"), "round " + round + " must not see a stale H19"); + } + } + + @Test + void allocation_indexArraysAreNotReallocatedOnceWarm() { + // The rigorous 0 B/op verification is the Phase 17 JMH gate (-prof gc); this is a + // unit-test-level structural guarantee that repeated first()/all()/view() lookups never + // re-trigger index growth (Arrays.copyOf inside ensureIndexCapacity) after the first + // reset() has already sized the arrays for this header count — asserted by identity: the + // backing array references must be the exact same objects before and after 100k lookups. + HeaderMap map = parse("A: 1", "B: 2", "C: 3", "D: 4"); + int[] namesBefore = arrayFieldValue(map, "nameOffsets"); + + for (int i = 0; i < 100_000; i++) { + assertEquals("2", map.first("B")); + assertNotNull(map.view("C")); + assertFalse(map.all("D").isEmpty()); + } + + int[] namesAfter = arrayFieldValue(map, "nameOffsets"); + assertSame(namesBefore, namesAfter, "lookups alone must never reallocate the index arrays"); + } + + @Test + void view_poolWraparound_aliasesAnEarlierReturnedView() { + // EX-05's documented hazard, demonstrated through the actual public API: HeaderMap's + // view() pool is sized 4 (VIEW_POOL_SIZE); a 5th call in the same request wraps around + // and silently repositions the object the 1st call returned. + dev.relism.fpr.core.ByteView v1 = null; + HeaderMap map = parse("A: 1", "B: 2", "C: 3", "D: 4", "E: 5"); + for (String name : new String[]{"A", "B", "C", "D"}) { + dev.relism.fpr.core.ByteView v = map.view(name); + if (v1 == null) v1 = v; + } + assertEquals('1', v1.byteAt(0)); // still "A"'s value — pool has not wrapped yet + dev.relism.fpr.core.ByteView v5 = map.view("E"); // 5th call — wraps back to v1's slot + assertSame(v1, v5, "the 5th view() call must reuse the 1st call's slice instance"); + assertEquals('5', v1.byteAt(0)); // v1 is now silently "E"'s value, not "A"'s + } + + private static int[] arrayFieldValue(HeaderMap map, String fieldName) { + try { + var field = HeaderMap.class.getDeclaredField(fieldName); + field.setAccessible(true); + return (int[]) field.get(map); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } +} diff --git a/flash/src/test/java/dev/relism/flash/models/PathParamsTest.java b/flash/src/test/java/dev/relism/flash/models/PathParamsTest.java index f6299eb..913f065 100644 --- a/flash/src/test/java/dev/relism/flash/models/PathParamsTest.java +++ b/flash/src/test/java/dev/relism/flash/models/PathParamsTest.java @@ -1,5 +1,6 @@ package dev.relism.flash.models; +import dev.relism.flash.routing.routers.fastpathrouter.FastPathViews; import dev.relism.fpr.core.ByteView; import org.junit.jupiter.api.Test; @@ -64,4 +65,33 @@ class PathParamsTest { PathParams params = of("/users/123", "userId", "123"); assertNull(params.view("unknown")); } + + @Test + void view_poolWraparound_aliasesAnEarlierReturnedView() { + // EX-05's pooled path only engages when `source` is array-backed (ArrayBackedByteView) — + // unlike of()'s plain inline ByteView (which exercises the non-pooled fallback, still + // correct but not the code path this test targets), use the same view type RequestParser + // actually produces. + String path = "/a/1/b/2/c/3/d/4/e/5"; + byte[] bytes = path.getBytes(StandardCharsets.UTF_8); + ByteView source = new FastPathViews.RequestByteView(bytes, 0, bytes.length); + String[] names = {"a", "b", "c", "d", "e"}; + int[] starts = new int[names.length]; + int[] lens = new int[names.length]; + String[] values = {"1", "2", "3", "4", "5"}; + for (int i = 0; i < names.length; i++) { + starts[i] = path.indexOf(values[i]); + lens[i] = values[i].length(); + } + PathParams params = new PathParams(source, names, starts, lens); + + ByteView v1 = params.view("a"); + params.view("b"); + params.view("c"); + params.view("d"); // pool size 4 — not wrapped yet + assertEquals('1', v1.byteAt(0)); + ByteView v5 = params.view("e"); // 5th call wraps back to v1's slot + assertSame(v1, v5); + assertEquals('5', v1.byteAt(0)); + } } diff --git a/flash/src/test/java/dev/relism/flash/models/QueryParamsFastPathTest.java b/flash/src/test/java/dev/relism/flash/models/QueryParamsFastPathTest.java new file mode 100644 index 0000000..60aa45b --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/models/QueryParamsFastPathTest.java @@ -0,0 +1,95 @@ +package dev.relism.flash.models; + +import dev.relism.flash.routing.routers.fastpathrouter.FastPathViews; +import dev.relism.fpr.core.ByteView; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-26}: the clean-value (no {@code %}/{@code +}) fast path in {@code QueryParams.decode} + * must produce byte-for-byte identical results to the percent-decoding slow path it bypasses — + * verified here across clean values, values needing every kind of decoding, and the boundary + * between them. Also covers {@code EX-05}'s pooled {@code view()}. + */ +class QueryParamsFastPathTest { + + private static QueryParams of(String query) { + byte[] bytes = query.getBytes(StandardCharsets.US_ASCII); + return new QueryParams(new FastPathViews.RequestByteView(bytes, 0, bytes.length)); + } + + @Test + void cleanValue_noPercentOrPlus_decodesToItself() { + QueryParams qp = of("name=hello&city=NewYork"); + assertEquals("hello", qp.get("name")); + assertEquals("NewYork", qp.get("city")); + } + + @Test + void valueWithPlus_decodesToSpace_takesSlowPath() { + QueryParams qp = of("q=hello+world"); + assertEquals("hello world", qp.get("q")); + } + + @Test + void valueWithPercentEscape_decodesCorrectly_takesSlowPath() { + QueryParams qp = of("q=hello%20world"); + assertEquals("hello world", qp.get("q")); + } + + @Test + void valueWithInvalidPercentEscape_keepsLiteralPercent() { + QueryParams qp = of("q=100%25off"); + assertEquals("100%off", qp.get("q")); + QueryParams qp2 = of("q=trailing%2"); + assertEquals("trailing%2", qp2.get("q")); + } + + @Test + void emptyValue_isClean_decodesToEmptyString() { + QueryParams qp = of("a=&b=1"); + assertEquals("", qp.get("a")); + assertEquals("1", qp.get("b")); + } + + @Test + void mixedCleanAndEncodedValues_inSameQueryString() { + QueryParams qp = of("clean=abc&encoded=a%20b&plussed=a+b"); + assertEquals("abc", qp.get("clean")); + assertEquals("a b", qp.get("encoded")); + assertEquals("a b", qp.get("plussed")); + } + + // ── EX-05: pooled view() ──────────────────────────────────────────────── + + @Test + void view_returnsRawUndecodedBytes() { + QueryParams qp = of("q=a+b"); + ByteView v = qp.view("q"); + assertNotNull(v); + assertEquals(3, v.length()); + assertEquals('+', (char) v.byteAt(1)); // raw, not percent/plus-decoded + } + + @Test + void view_missingKey_returnsNull() { + QueryParams qp = of("q=1"); + assertNull(qp.view("missing")); + } + + @Test + void view_poolWraparound_aliasesAnEarlierReturnedView() { + QueryParams qp = of("a=1&b=2&c=3&d=4&e=5"); + ByteView v1 = qp.view("a"); + qp.view("b"); + qp.view("c"); + qp.view("d"); // pool size 4 — not wrapped yet + assertEquals('1', v1.byteAt(0)); + ByteView v5 = qp.view("e"); // 5th call wraps back to v1's slot + assertSame(v1, v5); + assertEquals('5', v1.byteAt(0)); + } +} diff --git a/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java b/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java index 2acbfba..ede5000 100644 --- a/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java +++ b/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java @@ -17,7 +17,7 @@ class AbstractRouterTest { String lastAddedPath; @Override - public RequestHandler route(Request request) { return null; } + public RequestHandler route(Request request, Object scratch) { return null; } @Override protected AbstractRouter addRoute(HttpMethod method, String path, RequestHandler handler) { diff --git a/flash/src/test/java/dev/relism/flash/routing/AbstractWsRouterTest.java b/flash/src/test/java/dev/relism/flash/routing/AbstractWsRouterTest.java index 091f251..4105cc5 100644 --- a/flash/src/test/java/dev/relism/flash/routing/AbstractWsRouterTest.java +++ b/flash/src/test/java/dev/relism/flash/routing/AbstractWsRouterTest.java @@ -15,7 +15,7 @@ class AbstractWsRouterTest { String lastPath; @Override - public WebSocketHandler route(Request request) { return null; } + public WebSocketHandler route(Request request, Object scratch) { return null; } @Override protected AbstractWsRouter addRoute(HttpMethod method, String path, WebSocketHandler handler) { diff --git a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java index 5c97c17..c621367 100644 --- a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java +++ b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathRouterImplTest.java @@ -33,12 +33,13 @@ class FastPathRouterImplTest { FastPathRouterImpl router = new FastPathRouterImpl(); router.doRegister(HttpMethod.GET, "/a", new SimpleHandler((req, res) -> "A"), NO_MW); router.doRegister(HttpMethod.POST, "/b", new SimpleHandler((req, res) -> "B"), NO_MW); + Object scratch = router.newScratch(); - RequestHandler res1 = router.route(mockRequest(HttpMethod.GET, "/a")); + RequestHandler res1 = router.route(mockRequest(HttpMethod.GET, "/a"), scratch); assertNotNull(res1); assertEquals("A", res1.handle(null, null)); - RequestHandler res2 = router.route(mockRequest(HttpMethod.POST, "/b")); + RequestHandler res2 = router.route(mockRequest(HttpMethod.POST, "/b"), scratch); assertNotNull(res2); assertEquals("B", res2.handle(null, null)); } @@ -47,9 +48,10 @@ class FastPathRouterImplTest { void route_noMatch_returnsNull() { FastPathRouterImpl router = new FastPathRouterImpl(); router.doRegister(HttpMethod.GET, "/a", new SimpleHandler((req, res) -> "A"), NO_MW); + Object scratch = router.newScratch(); - assertNull(router.route(mockRequest(HttpMethod.GET, "/b"))); - assertNull(router.route(mockRequest(HttpMethod.POST, "/a"))); + assertNull(router.route(mockRequest(HttpMethod.GET, "/b"), scratch)); + assertNull(router.route(mockRequest(HttpMethod.POST, "/a"), scratch)); } @Test @@ -59,7 +61,7 @@ class FastPathRouterImplTest { new SimpleHandler((req, res) -> "Extract"), NO_MW); Request request = mockRequest(HttpMethod.GET, "/users/123/items/456"); - RequestHandler handler = router.route(request); + RequestHandler handler = router.route(request, router.newScratch()); assertNotNull(handler); assertEquals("Extract", handler.handle(request, null)); @@ -67,4 +69,35 @@ class FastPathRouterImplTest { assertEquals("123", request.param("id")); assertEquals("456", request.param("itemId")); } + + @Test + void route_reusesScratchAcrossManyRequests_includingGrowingParamCapacity() throws Exception { + // EX-19: the same scratch, reused across a mix of param counts, must keep matching + // correctly as its arrays grow past their initial size (8) and get reused afterward. + FastPathRouterImpl router = new FastPathRouterImpl(); + router.doRegister(HttpMethod.GET, "/a/{p1}/{p2}/{p3}/{p4}/{p5}/{p6}/{p7}/{p8}/{p9}/{p10}", + new SimpleHandler((req, res) -> "many"), NO_MW); + router.doRegister(HttpMethod.GET, "/b/{id}", new SimpleHandler((req, res) -> "one"), NO_MW); + Object scratch = router.newScratch(); + + for (int i = 0; i < 3; i++) { + Request oneParam = mockRequest(HttpMethod.GET, "/b/123"); + assertEquals("one", router.route(oneParam, scratch).handle(oneParam, null)); + assertEquals("123", oneParam.param("id")); + + Request tenParams = mockRequest(HttpMethod.GET, "/a/1/2/3/4/5/6/7/8/9/10"); + assertEquals("many", router.route(tenParams, scratch).handle(tenParams, null)); + assertEquals("10", tenParams.param("p10")); + assertEquals("1", tenParams.param("p1")); + + // The 1-param request that follows a 10-param one must not see stale params left + // over from the larger match in the shared, oversized arrays. + Request oneParamAgain = mockRequest(HttpMethod.GET, "/b/456"); + RequestHandler h = router.route(oneParamAgain, scratch); + assertNotNull(h); + h.handle(oneParamAgain, null); + assertEquals("456", oneParamAgain.param("id")); + assertNull(oneParamAgain.param("p10")); + } + } } diff --git a/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsLongAtTest.java b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsLongAtTest.java new file mode 100644 index 0000000..47cbec9 --- /dev/null +++ b/flash/src/test/java/dev/relism/flash/routing/routers/fastpathrouter/FastPathViewsLongAtTest.java @@ -0,0 +1,116 @@ +package dev.relism.flash.routing.routers.fastpathrouter; + +import dev.relism.fpr.core.FastPathRouter; +import dev.relism.fpr.core.MatchResult; +import dev.relism.fpr.core.RouterBuilder; +import dev.relism.fpr.core.dsl.StringRouteParser; +import dev.relism.fpr.core.internal.runtime.ByteCompare; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * {@code EX-04}: verifies the {@code longAt()}/{@code supportsLong()} contract against + * {@code fpr-core}'s own word-at-a-time comparison code — not merely against a hand-derived + * expectation, per the plan's explicit instruction to verify by testing against {@code fpr-core} + * directly rather than by reading its bytecode (bytecode-reading only informed which byte order + * to use; this test is the actual verification). A wrong endianness or a wrong bounds assumption + * here produces silently mis-routed requests, the worst possible failure mode ({@code EX-04}'s + * own registry entry) — so this covers both the raw word-read contract and an end-to-end router + * match with the long path actually engaged. + */ +class FastPathViewsLongAtTest { + + // ── Raw longAt() vs. a hand-assembled little-endian expectation ──────── + + @Test + void longAt_assemblesLittleEndian() { + byte[] buf = {1, 2, 3, 4, 5, 6, 7, 8}; + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(buf, 0, 8); + assertTrue(view.supportsLong()); + long expected = 0x0807060504030201L; // byte 0 -> least significant byte + assertEquals(expected, view.longAt(0)); + } + + @Test + void longAt_respectsViewOffset_notJustArrayOffset() { + byte[] buf = {(byte) 0xFF, (byte) 0xFF, 1, 2, 3, 4, 5, 6, 7, 8, (byte) 0xFF}; + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(buf, 2, 8); + long expected = 0x0807060504030201L; + assertEquals(expected, view.longAt(0)); + } + + // ── Cross-checked against fpr-core's own ByteCompare, the actual consumer of longAt() ── + + @Test + void byteCompareEquals_agreesBetweenLongPathAndByteAtATimePath_onIdenticalContent() { + byte[] content = "GET/users/1234567890/profile".getBytes(StandardCharsets.US_ASCII); + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(content, 0, content.length); + byte[] other = content.clone(); + + assertTrue(ByteCompare.equals(view, 0, other, 0, content.length, true)); + assertTrue(ByteCompare.equals(view, 0, other, 0, content.length, false)); + } + + @Test + void byteCompareEquals_agreesBetweenLongPathAndByteAtATimePath_onDivergingContent() { + // Diverge at every position across an 8+-byte range, including inside a word, at a word + // boundary, and in the scalar tail — a wrong longAt() would only show up at some of these. + byte[] base = "abcdefghijklmnopqrstuvwxyz012345".getBytes(StandardCharsets.US_ASCII); + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(base, 0, base.length); + for (int diffAt = 0; diffAt < base.length; diffAt++) { + byte[] other = base.clone(); + other[diffAt] = (byte) (other[diffAt] + 1); + boolean withLong = ByteCompare.equals(view, 0, other, 0, base.length, true); + boolean withoutLong = ByteCompare.equals(view, 0, other, 0, base.length, false); + assertFalse(withLong, "long path failed to detect divergence at " + diffAt); + assertEquals(withoutLong, withLong, "long/byte-at-a-time paths disagree at diffAt=" + diffAt); + } + } + + @Test + void byteCompareIndexOf_agreesBetweenLongPathAndByteAtATimePath() { + byte[] haystack = "xxxxxxxxxxxxxxxxxTARGETxxxxxxxxxxxxxxxxxx".getBytes(StandardCharsets.US_ASCII); + byte[] needle = "TARGET".getBytes(StandardCharsets.US_ASCII); + FastPathViews.RequestByteView view = new FastPathViews.RequestByteView(haystack, 0, haystack.length); + + int withLong = ByteCompare.indexOf(view, 0, haystack.length, needle, 0, needle.length, true); + int withoutLong = ByteCompare.indexOf(view, 0, haystack.length, needle, 0, needle.length, false); + assertEquals(withoutLong, withLong); + assertTrue(withLong >= 0); + } + + // ── End-to-end: a real router, literal routes >= 8 bytes, long path actually engaged ──── + + @Test + void router_matchesCorrectly_withLongLiteralSegmentsAndTheLongPathEnabled() { + RouterBuilder builder = new RouterBuilder<>(); + builder.add(StringRouteParser.parse("GET/aaaaaaaaaaaaaaaaaaaa"), "route-a"); + builder.add(StringRouteParser.parse("GET/bbbbbbbbbbbbbbbbbbbb"), "route-b"); + builder.add(StringRouteParser.parse("GET/aaaaaaaaaaaaaaaaaaab"), "route-a-near-miss"); + FastPathRouter router = builder.compile(); + + assertEquals("route-a", matchOne(router, "GET", "/aaaaaaaaaaaaaaaaaaaa")); + assertEquals("route-b", matchOne(router, "GET", "/bbbbbbbbbbbbbbbbbbbb")); + // Differs only in the very last byte — must not be conflated with route-a by a + // word-at-a-time comparison that got the tail handling wrong. + assertEquals("route-a-near-miss", matchOne(router, "GET", "/aaaaaaaaaaaaaaaaaaab")); + assertNull(matchOne(router, "GET", "/aaaaaaaaaaaaaaaaaaac")); + assertNull(matchOne(router, "GET", "/ccccccccccccccccccccc")); + } + + private static String matchOne(FastPathRouter router, + String method, String path) { + byte[] methodBytes = method.getBytes(StandardCharsets.US_ASCII); + FastPathViews.RequestByteView pathView = + new FastPathViews.RequestByteView(path.getBytes(StandardCharsets.US_ASCII), 0, path.length()); + FastPathViews.MethodPathByteView combined = new FastPathViews.MethodPathByteView(); + combined.reset(methodBytes, pathView); + + MatchResult result = new MatchResult<>(8, 32); + int labelId = router.match(combined, result); + return labelId == FastPathRouter.NO_MATCH ? null : result.handler(); + } +}