Go version
go version go1.27.2 darwin/arm64
Output of go env in your module/workspace:
AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN='/Users/ville/go/bin'
GOCACHE='/Users/ville/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/ville/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/p3/2xkq3ckn3hj3tbdbkyqn03rm0000gn/T/go-build1420015243=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/ville/git/x/net/go.mod'
GOMODCACHE='/Users/ville/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPACKAGESDRIVER=''
GOPATH='/Users/ville/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.27.2/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/ville/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.27.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.27.2'
GOWORK=''
PKG_CONFIG='pkg-config'
What did you do?
I profiled the existing x/net/html benchmarks. Then looked at outliers in allocation sites by source line with pprofecy.
What did you see happen?
In BenchmarkLowLevelTokenizer over 80% of the allocations from bytes.ReplaceAll in TagName and TagAttr:
alloc_objects · total 4.91 M · focus bytes.Replace 4.01 M (81.6%) · callers · min 1%
└─ bytes.Replace bytes.go:1162 4.01 M 81.6%
└─ bytes.ReplaceAll: return Replace(s, old, new, -1) :1207 4.01 M 81.6%
├─ html.(*Tokenizer).TagName: s := bytes.ReplaceAll(z.buf[z.data.start:z.data… token.go:1227 3.47 M 70.8%
│ ├─ html.benchmarkTokenizer: _, more := z.TagName() token_test.go:951 1.54 M ─┐
│ └─ html.benchmarkTokenizer: z.TagName() token_test.go:956 1.93 M ─┴─ 3.47 M 70.8% (2 calls)
│ └─ html.benchmarkTokenizer [+] token_test.go:920
├─ html.(*Tokenizer).TagAttr: key = bytes.ReplaceAll(z.buf[x[0].start:x[0].en… token.go:1245 0.26 M ─┐
└─ html.(*Tokenizer).TagAttr: val = bytes.ReplaceAll(z.buf[x[1].start:x[1].en… token.go:1246 0.27 M ─┴─ 0.53 M 10.8% (2 calls)
└─ html.(*Tokenizer).TagAttr token.go:1239
└─ html.benchmarkTokenizer: _, _, more = z.TagAttr() [+] token_test.go:953 0.53 M 10.8%
The problem is that bytes.ReplaceAll returns a copy, even when the input contains no NUL bytes, for every TagName and TagAttr.
The implementation was changed in CL 781883 (html: replace nul bytes in tag names and attributes) which was released in v0.56.0.
What did you expect to see?
TagName and TagAttr should only copy when there is a NUL byte to replace. The Text() implementation in the same package already does this for text and comment tokens:
if (z.convertNUL || z.tt == CommentToken) && bytes.Contains(s, nul) {
s = bytes.Replace(s, nul, replacement, -1)
}Here's the benchstat output with the proposed fix, compared to current master at c1d31df.
Details
goos: darwin
goarch: arm64
pkg: golang.org/x/net/html
cpu: Apple M1 Max
│ master.txt │ new.txt │
│ sec/op │ sec/op vs base │
Parser-10 820.1µ ± 0% 761.2µ ± 0% -7.19% (p=0.000 n=10)
RawLevelTokenizer-10 276.3µ ± 0% 277.0µ ± 1% ~ (p=0.190 n=10)
LowLevelTokenizer-10 437.9µ ± 0% 371.0µ ± 1% -15.27% (p=0.000 n=10)
HighLevelTokenizer-10 516.0µ ± 1% 451.1µ ± 2% -12.59% (p=0.000 n=10)
geomean 475.7µ 433.4µ -8.89%
│ master.txt │ new.txt │
│ B/s │ B/s vs base │
Parser-10 90.89Mi ± 0% 97.92Mi ± 0% +7.74% (p=0.000 n=10)
RawLevelTokenizer-10 269.8Mi ± 0% 269.1Mi ± 1% ~ (p=0.190 n=10)
LowLevelTokenizer-10 170.2Mi ± 0% 200.9Mi ± 1% +18.03% (p=0.000 n=10)
HighLevelTokenizer-10 144.4Mi ± 1% 165.2Mi ± 2% +14.40% (p=0.000 n=10)
geomean 156.7Mi 172.0Mi +9.75%
│ master.txt │ new.txt │
│ B/op │ B/op vs base │
Parser-10 575.4Ki ± 0% 539.6Ki ± 0% -6.22% (p=0.000 n=10)
RawLevelTokenizer-10 10.23Ki ± 0% 10.23Ki ± 0% ~ (p=1.000 n=10) ¹
LowLevelTokenizer-10 42.29Ki ± 0% 10.23Ki ± 0% -75.80% (p=0.000 n=10)
HighLevelTokenizer-10 128.15Ki ± 0% 92.36Ki ± 0% -27.93% (p=0.000 n=10)
geomean 75.16Ki 47.80Ki -36.40%
¹ all samples are equal
│ master.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Parser-10 11.189k ± 0% 7.470k ± 0% -33.24% (p=0.000 n=10)
RawLevelTokenizer-10 678.0 ± 0% 678.0 ± 0% ~ (p=1.000 n=10) ¹
LowLevelTokenizer-10 4397.0 ± 0% 678.0 ± 0% -84.58% (p=0.000 n=10)
HighLevelTokenizer-10 7.094k ± 0% 3.375k ± 0% -52.42% (p=0.000 n=10)
geomean 3.922k 1.845k -52.96%
Go version
go version go1.27.2 darwin/arm64
Output of
go envin your module/workspace:What did you do?
I profiled the existing
x/net/htmlbenchmarks. Then looked at outliers in allocation sites by source line with pprofecy.What did you see happen?
In
BenchmarkLowLevelTokenizerover 80% of the allocations frombytes.ReplaceAllinTagNameandTagAttr:The problem is that
bytes.ReplaceAllreturns a copy, even when the input contains no NUL bytes, for everyTagNameandTagAttr.The implementation was changed in CL 781883 (
html: replace nul bytes in tag names and attributes) which was released inv0.56.0.What did you expect to see?
TagNameandTagAttrshould only copy when there is a NUL byte to replace. TheText()implementation in the same package already does this for text and comment tokens:Here's the benchstat output with the proposed fix, compared to current master at c1d31df.
Details