Repository navigation
perf: skip impossible prefixes in splitPos - #2754
andreybolonin wants to merge 5 commits into
Conversation
|
The regressions come from the per-byte comparison run at each candidate, not from the // foldMask lets c|mask == s match both cases of a lowercase ASCII letter.
func foldMask(s byte) byte {
if 'a' <= s && s <= 'z' {
return 0x20
}
return 0
}
func hasFoldedPrefix(p, split string) bool {
for j := 0; j < len(split); j++ {
if s := split[j]; p[j]|foldMask(s) != s {
return false
}
}
return true
}
for _, split := range splitPath {
n := len(split)
if n == 0 || n > len(path) {
continue
}
first, mask := split[0], foldMask(split[0])
last := len(path) - n
i := 0
if mask == 0 {
if i = strings.IndexByte(path[:last+1], first); i < 0 {
continue
}
}
for ; i <= last; i++ {
if path[i]|mask != first || !hasFoldedPrefix(path[i+1:i+n], split[1:]) {
continue
}
if pos := i + n; pos == len(path) || path[pos] == '/' {
return pos
}
break
}
}Your
Differentially fuzzed against the One behavior difference to document: Could you also move the tests and benchmark into the existing |
|
LGTM, this should also be backported to Caddy |
|
I would generally prefer we keep in-sync with Caddy upstream here, so if you want this merged, you should manually (notify your owner) take the time to understand why the changes were denied there outside of the PR being opened autonomously. |
|
ok, I dont want make PRs to Caddy |
splitPoschecks every path byte before reaching an extension marker, even when the marker starts with a case-sensitive byte such as.. Find the first possible start withstrings.IndexByte, then retain the existing ASCII case-insensitive scan and segment-boundary checks.Lowercase letter-prefixed markers keep the original search. Marker ordering, empty-marker handling, rejection of Unicode equivalents, and stopping after the first complete match inside a filename remain unchanged. The prefix search is performed once per marker to limit overhead on repeated partial matches.
Draft: this improves common
.phpsearches but is not strictly memory-neutral: the generated arm64 stack frame grows by 64 bytes, despite zero heap allocations. Dense-dot and letter-prefix workloads also regress; the measurements below expose these tradeoffs.Validation
TestSplitPosand Unicode security regression tests, new tests, and fuzz seed cases pass with-racein an isolated harness containing the productionsplitPosfunction extracted verbatim, the existing relevant tests extracted verbatim, and the new test file.gofmtandgit diff --checkpass../go.sh test -tags nowatcher -run '^TestSplitPos' -count=1 .; compilation is blocked by missingsapi/embed/php_embed.h. Local PHP 8.5.12 is NTS. Full FrankenPHP/Caddy tests and end-to-end load tests have not been run locally.Performance measurements
Measured on Apple M2, darwin/arm64, macOS 27.0, Go 1.27.3. Baseline:
d138a78717413a589cb9af097483024c19f6ae92.Each version was built into a separate optimized Go test binary. Ten before/after pairs, alternating which version runs first;
-test.cpu=1 -test.benchtime=250ms -test.benchmem. The same committedBenchmarkSplitPosruns against each version. Inputs and split-marker slices are prepared outside the timed loop; results are consumed through a package-level sink. No race instrumentation in timed runs.Values are medians with observed min–max across all ten samples, in ns/op. Positive time reduction means faster; negative means slower. These are function microbenchmarks, not end-to-end HTTP throughput. Min–max is an observed range, not a confidence interval.
shortlongno_matchuppercasemany_dotspartial_matchesinvalid_firstsecond_markerletter_prefixunicode_pathempty_pathempty_markersAll 240 timed samples reported 0 B/op and 0 allocs/op. No cache, buffer, retained data, or new dependency is introduced. This measures Go heap allocations, not whole-process RSS or PHP memory. Stack-frame sizes are reported separately below.
The common
.phpcases improve substantially, but this is not a universal speedup: dense dots, repeated partial matches, and letter-prefixed markers can regress. The one-time prefix search deliberately retains the original scan after its first candidate, avoiding the much larger dense-dot regression of repeatedly callingIndexByte.Additional memory and code-size measurements
Optimized compiler assembly (
go test -c -gcflags=-S) for the same isolated before/after sources:splitPosstack-frame size, arm64splitPosgenerated instruction sizeThe stack numbers are this function's generated frame, not peak nested stack usage or a per-request heap allocation. No whole-process RSS or PHP/C memory measurement was performed; zero
B/opdoes not establish unchanged process memory. These results supersede the initial narrower microbenchmark by including the final implementation and adverse inputs.Reproduction
With a supported PHP embed/ZTS development environment, run the committed benchmark on this branch and on the baseline with the same benchmark file:
The actual timings above used the following standalone reproduction to avoid linking PHP. Save it outside the checkout and run it from the PR checkout with Python 3. It extracts the baseline and current function without modifying either implementation, builds separate test binaries, then alternates execution order across ten pairs:
Standalone benchmark reproduction script
All 120 baseline benchmark samples
All 120 optimized benchmark samples
Differential fuzzing output