btf: add essentialNameLen() to handle flavor underscores properly - #2090
btf: add essentialNameLen() to handle flavor underscores properly#2090Asphaltt wants to merge 1 commit into
Conversation
78578c8 to
a87ccb2
Compare
| return essentialName(name) | ||
|
|
||
| return len(name) | ||
| } |
There was a problem hiding this comment.
I agree with you that looking for a non _ before and after the ___ is more correct.
However, bytes.LastIndex is a very optimized function, using a string search algorithm and perhaps even CPU specific optimizations. The new implementation does not.
This results in a 30% slowdown in vmlinux parsing.
benchstat before.txt after.txt
goos: linux
goarch: amd64
pkg: github.com/cilium/ebpf/btf
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
│ before.txt │ after.txt │
│ sec/op │ sec/op vs base │
ParseVmlinux-20 7.472m ± 5% 9.843m ± 1% +31.74% (p=0.000 n=20)
│ before.txt │ after.txt │
│ B/op │ B/op vs base │
ParseVmlinux-20 1.254Mi ± 0% 1.254Mi ± 0% ~ (p=0.310 n=20)
│ before.txt │ after.txt │
│ allocs/op │ allocs/op vs base │
ParseVmlinux-20 246.0 ± 0% 246.0 ± 0% ~ (p=1.000 n=20) ¹
¹ all samples are equal
We should investigate to see if we can come up with a better implementation. Perhaps its better to call strings.LastIndex in a loop and to add validation for each find. Perhaps some form of string search algorithm
There was a problem hiding this comment.
Tried to optimise it by utilising bytes.LastIndex():
benchstat before.txt after.txt scan.txt
goos: linux
goarch: amd64
pkg: github.com/cilium/ebpf/btf
cpu: Intel(R) Xeon(R) Silver 4116 CPU @ 2.10GHz
│ before.txt │ after.txt │ scan.txt │
│ sec/op │ sec/op vs base │ sec/op vs base │
ParseVmlinux-48 13.91m ± 0% 14.15m ± 0% +1.69% (p=0.000 n=20) 15.98m ± 0% +14.85% (p=0.000 n=20)
│ before.txt │ after.txt │ scan.txt │
│ B/op │ B/op vs base │ B/op vs base │
ParseVmlinux-48 1.254Mi ± 0% 1.254Mi ± 0% ~ (p=0.683 n=20) 1.254Mi ± 0% ~ (p=0.954 n=20)
│ before.txt │ after.txt │ scan.txt │
│ allocs/op │ allocs/op vs base │ allocs/op vs base │
ParseVmlinux-48 246.0 ± 0% 246.0 ± 0% ~ (p=1.000 n=20) ¹ 246.0 ± 0% ~ (p=1.000 n=20) ¹
¹ all samples are equal
before.txt: Before this PR.after.txt: With the optimisation.scan.txt: Without the optimisation.
With the optimisation, it looks much better now. Even though, the optimisation cannot eliminate the overhead.
Could you verify the benchmark again?
a87ccb2 to
50cc07b
Compare
Only treat triple underscores as a CO-RE flavor separator when surrounded by non-underscore characters. This keeps names like ____fput distinct while using the same normalization for lookup and decoder indexing. Add tests for leading underscore runs and valid flavor suffixes. See libbpf's bpf_core_essential_name_len(). Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
50cc07b to
f7bc841
Compare
#2086 didn't fix the flavor underscores issue thoroughly.
By referencing libbpf's bpf_core_essential_name_len(), here's my fix.
Only treat triple underscores as a CO-RE flavor separator when surrounded by non-underscore characters. This keeps names like ____fput distinct while using the same normalization for lookup and decoder indexing.
Add tests for leading underscore runs and valid flavor suffixes.