perf: avoid string reversal allocation in DomainSet.Has

[Ember-Moth]

Problem

DomainSet.Has normalizes every query with two string conversions before walking the trie:

key = utils.Reverse(key)
key = strings.ToLower(key)

utils.Reverse converts the key to []rune and back to string, so each lookup pays a rune decode, a reverse copy and a re-encode. This sits on the hot path of every domain match: GEOSITE rules (component/geodata/router/condition.go), RULE-SET domain matching (rules/provider/domain_strategy.go), the fake-ip skipper (component/fakeip/skipper.go) and the sniffer dispatcher (component/sniffer/dispatcher.go).

The cost is length dependent. The Go compiler keeps a non-escaping conversion off the heap only while it fits its 32-byte implicit temporary, so hostnames longer than that — most CDN and cloud endpoints — allocate twice per lookup: 244 B/op and 2 allocs/op, against 6 B/op and 0 allocs/op for short keys.

Change

The trie stores reversed keys, but the reversal only has to be observable inside the matching loop. Has now reads the key back to front through a small inlinable accessor, so no buffer is reserved and nothing is copied:

func revLowerAt(key string, i int) byte {
	c := key[len(key)-1-i]
	if c >= 'A' && c <= 'Z' {
		c += 'a' - 'A'
	}
	return c
}

Keys containing a byte >= utf8.RuneSelf rewrite key through strings.ToLower(utils.Reverse(key)), byte-reversed so the accessor observes it unchanged, then fall into the same loop. That path is required for correctness, not caution: NewDomainSet builds the set with rune-wise reversal, which only equals byte-wise reversal for ASCII.

No exported signature, no DomainSet field, and no change to the MRS binary format written by WriteBin / read by ReadDomainSetBin.

Commits, after review: the first used a fixed 255-byte stack buffer, the second removed it, and the third folded the matching loop back into Has now that there is a single path again.

make([]byte, len(key)) was measured as an alternative to the fixed buffer and is not allocation free — escape analysis reports does not escape, but the generated code still branches to runtime.makeslice once the length exceeds 32 bytes, which costs 48 B/op and 1 allocs/op on long keys. Reading in place avoids both the reserved frame and the allocation.

Benchmark

BenchmarkDomainSetHas is added in this PR: 10k suffix rules, keys mixing exact hits, subdomain hits, misses and uppercase input, split by length because of the 32-byte threshold above. benchstat, -benchtime=1s -count=10, linux/amd64:

                      │    alpha    │               in place              │
                      │   sec/op    │   sec/op     vs base                │
DomainSetHas/short-16   280.1n ± 4%   171.7n ± 4%  -38.70% (p=0.000 n=10)
DomainSetHas/long-16    469.6n ± 2%   180.9n ± 4%  -61.48% (p=0.000 n=10)
geomean                 362.7n        176.2n       -51.41%

	alpha	in place
short B/op, allocs/op	6, 0	0, 0
long B/op, allocs/op	244, 2	0, 0

To reproduce both sides, keeping the new benchmark and swapping only the implementation:

go test ./component/trie/ -run xxx -bench BenchmarkDomainSetHas -benchtime=1s -count=10
git checkout Alpha -- component/trie/domain_set.go
go test ./component/trie/ -run xxx -bench BenchmarkDomainSetHas -benchtime=1s -count=10
git checkout HEAD -- component/trie/domain_set.go

Tests

Added coverage the existing tests did not have:

• TestDomainSetCase — uppercase queries against lowercase rules.
• TestDomainSetUnicode — non-ASCII rules and queries, exercising the rune-wise fallback.
• TestDomainSetOversizedKey — a key longer than any valid domain name.

These three tests also pass against an unmodified domain_set.go, which is deliberate: they pin existing behaviour rather than describe the new implementation.

Match counts were also compared against the current implementation over a real geosite.dat, using 20k mixed hit/miss queries per category: geosite:cn (112,332 rules) 27,185 hits, geosite:google 5,345, geosite:apple 8,935 — identical on both sides.

Notes

Foreach still calls utils.Reverse. That is a dump path rather than a lookup path, so it is left alone to keep this change focused.

[wwqgtxx]

That’s an interesting task.

However, you should verify whether directly using make([]byte, len(key)) with an appropriate size actually triggers heap allocation; given the Go compiler's implementation, it shouldn't—provided escape analysis works as expected. This approach would make the code more robust by avoiding the need to always allocate 253 bytes of stack space.

[Ember-Moth]

Thanks — the stack space concern is fair, and chasing it led somewhere better than either option, because make([]byte, len(key)) turns out not to be allocation free.

Escape analysis does agree with you:

domain_set.go:86:14: make([]byte, len(key)) does not escape

But "does not escape" is not the same as "stays on the stack". The generated code only keeps it off the heap while the length fits the compiler's 32-byte implicit temporary:

CMPQ  CX, $32
JHI   89                       ; longer than 32 -> heap path
LEAQ  autotmp_18+192(SP), DX   ; 32-byte stack temp
MOVUPS X15, (DX)
MOVUPS X15, 16(DX)
...
CALL  runtime.makeslice(SB)    ; domain_set.go:86

So any hostname over 32 bytes — ec2-52-201-13-44.compute-1.example.com, most CDN and cloud endpoints — takes the runtime.makeslice path and allocates.

That also exposed a mistake in my benchmark: every key in it was 20–24 bytes, so it never left the stack path. It understated the problem as well — the current code allocates 244 B/op and 2 allocs/op on keys over 32 bytes, not the 6 B/op the short-key benchmark suggested. The benchmark now covers both lengths.

Rather than trade a 255-byte frame against an allocation, the second commit removes the buffer altogether. has reads the key back to front through a small revLowerAt accessor, so nothing is reserved and nothing is copied. Non-ASCII keys still normalize through the rune-wise reversal the set is built with, byte-reversed so the accessor observes it unchanged.

benchstat, -benchtime=1s -count=10, linux/amd64, against current Alpha:

                      │    alpha    │              fixed buffer           │             make([]byte,n)          │              in place               │
                      │   sec/op    │   sec/op     vs base                │   sec/op     vs base                │   sec/op     vs base                │
DomainSetHas/short-16   280.1n ± 4%   175.9n ± 4%  -37.18% (p=0.000 n=10)   181.5n ± 6%  -35.20% (p=0.000 n=10)   171.7n ± 4%  -38.70% (p=0.000 n=10)
DomainSetHas/long-16    469.6n ± 2%   191.7n ± 3%  -59.17% (p=0.000 n=10)   220.3n ± 3%  -53.08% (p=0.000 n=10)   180.9n ± 4%  -61.48% (p=0.000 n=10)
geomean                 362.7n        183.7n       -49.36%                  200.0n       -44.86%                  176.2n       -51.41%

	alpha	fixed buffer	make([]byte, n)	in place
short B/op, allocs/op	6, 0	0, 0	0, 0	0, 0
long B/op, allocs/op	244, 2	0, 0	48, 1	0, 0
stack reserved	—	255 B	32 B + heap fallback	none

No buffer, no allocation at any key length, and slightly faster than the fixed buffer.

On correctness: the three added tests still pass against an unmodified domain_set.go, and match counts are identical to the current implementation across geosite:cn (112,332 rules), geosite:google and geosite:apple from a real geosite.dat — 27,185 / 5,345 / 8,935 hits over 20k mixed hit/miss queries.

[wwqgtxx]

Based on the current code structure, a lowercase has function should no longer be needed; the utf8 branch can simply replace the key and then break.

[Ember-Moth]

Done — the non-ASCII branch now rewrites key and breaks, so Has has a single path and the has helper is gone. Net -3 lines and one less level of indirection.

Benchmarks are unchanged, so the split was not buying anything:

                      │   with has()   │              folded into Has        │
                      │    sec/op      │   sec/op     vs base                │
DomainSetHas/short-16    173.8n ± 4%     174.6n ± 3%       ~ (p=0.869 n=10)
DomainSetHas/long-16     177.9n ± 4%     180.4n ± 1%       ~ (p=1.000 n=10)

revLowerAt and byteReverse are kept as helpers below Has. Tests still pass, and match counts over a real geosite.dat are unchanged (27,185 / 5,345 / 8,935 for cn / google / apple). The PR description is updated to match.