Skip to content

Commit 37ee732

Browse files
etrclaude
andcommitted
TASK-058: housekeeping -- check off action items, mark Done
All four sub-items implemented and committed: step 0 (commit 18693a5): bench harness + baseline numbers step 1 (commit e2f730d): canonicalize_lookup_path returns string_view step 2 (commit 51b5a37): pre-normalize auth_skip_paths + early-out step 3 (commit b22b0dd): lazy Allow-header cache on http_resource Two interpretive deviations from the brief recorded inline in the action items (auth_skip_paths is a webserver-level filter, not a route_entry attribute; the Allow cache attaches to http_resource so mask mutation via set_allowing / disallow_all / allow_all invalidates implicitly). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent b22b0dd commit 37ee732

1 file changed

Lines changed: 34 additions & 4 deletions

File tree

specs/tasks/v2-deferred-backlog-plan.md

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ aggregation pipeline.
336336
**Milestone:** post-v2.0 (polish; not a release blocker)
337337
**Component:** `webserver_dispatch` / `webserver_request`
338338
**Estimate:** L (broken into ~4 sub-items)
339+
**Status:** Done
339340

340341
**Goal:**
341342
Three perf items deferred during the manual-validation sweep all share
@@ -344,21 +345,50 @@ the same shape — a `std::string` is rebuilt on every request when a
344345
release-blocking but together they're worth 5–15% on the warm path.
345346

346347
**Action Items:**
347-
- [ ] **canonicalize_lookup_path → string_view return.** Refactor
348+
- [x] **canonicalize_lookup_path → string_view return.** Refactor
348349
`src/detail/webserver_dispatch.cpp::canonicalize_lookup_path` to
349350
return a `string_view` into a caller-owned scratch buffer (or the
350351
request arena) instead of allocating a `std::string` on every call.
351-
- [ ] **normalize_path at registration time.** Move the
352+
Done as step 1 (commit `e2f730d`): canonical-input fast path returns
353+
the input view unchanged (zero allocation); non-canonical inputs
354+
write into a caller-owned scratch `std::string`. Direct unit pin in
355+
`test/unit/route_lookup_canonicalize_test.cpp`.
356+
- [x] **normalize_path at registration time.** Move the
352357
`normalize_path` call from `webserver_request.cpp::should_skip_auth`
353358
(where it runs per request) to the registration site, storing the
354359
normalized form in `route_entry`. Update the lookup to use the
355360
pre-normalized form.
356-
- [ ] **serialize_allow_methods caching.** `webserver_dispatch.cpp:363`
361+
Done as step 2 (commit `51b5a37`). Interpretive deviation noted in
362+
the commit message: the brief's `route_entry` framing doesn't map
363+
onto `should_skip_auth`, which compares the request path against a
364+
webserver-level `auth_skip_paths` list (not a per-route attribute);
365+
the optimisation moves to the data's actual home —
366+
`auth_skip_paths_normalized_` populated once at webserver
367+
construction — plus an empty-list early-out for the production-
368+
typical case. The empty-list early-out drops the per-request cost
369+
from ~620 ns to ~3 ns. Bug-fix bonus: non-canonical skip-list
370+
entries (`"/public/"`, `"/a/../b"`) now match canonical request paths.
371+
Pinned in `test/unit/auth_skip_normalize_test.cpp`.
372+
- [x] **serialize_allow_methods caching.** `webserver_dispatch.cpp`
357373
rebuilds the Allow header value on every 405 response. Cache the
358374
serialized string on `route_entry` and invalidate on
359375
add/remove-method.
360-
- [ ] Add `test/bench_warm_path.cpp` measuring before/after per-request
376+
Done as step 3 (commit `b22b0dd`). Interpretive deviation noted in
377+
the commit message: the brief's `route_entry` framing would go stale
378+
on `set_allowing` because `route_entry::methods` is the *registered*
379+
mask, not the resource's runtime mask; the cache attaches to
380+
`http_resource` instead (the same object that owns the mask) and
381+
invalidates implicitly by comparing the live mask against a snapshot
382+
at cache-fill time. Per-resource `std::mutex`. Pinned in
383+
`test/unit/http_resource_allow_cache_test.cpp` (correctness +
384+
identity / cache-hit-returns-same-buffer pin).
385+
- [x] Add `test/bench_warm_path.cpp` measuring before/after per-request
361386
cost so the next reviewer can verify the gain.
387+
Done as step 0 (commit `18693a5`). Wired into `bench_targets` in
388+
`test/Makefile.am` (not part of `make check`). Four measurements:
389+
`canonicalize`, `should_skip_auth` (non-empty + empty), and
390+
`serialize_allow_405`. Baseline numbers captured in the step-0
391+
commit body; step-3 commit body holds the after-each-step deltas.
362392

363393
**Dependencies:**
364394
- Blocked by: TASK-053 (must be the only caller of `canonicalize_lookup_path`)

0 commit comments

Comments
 (0)