8000 fix: improve performance of readStringVarFromBuff by geyslan · Pull Request #4194 · aquasecurity/tracee · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: improve performance of readStringVarFromBuff #4194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 23, 2024
Merged

fix: improve performance of readStringVarFromBuff #4194

merged 1 commit into from
Aug 23, 2024

Conversation

geyslan
Copy link
Member
@geyslan geyslan commented Jul 18, 2024 •
edited
Loading

Close: #4081

1. Explain what the PR does

28ec953 fix: improve performance of readStringVarFromBuff

Optimize readStringVarFromBuff for better performance and memory usage.

- The optimizations were tested using new benchmarks:
  - Short: 33.26% faster, 42.86% less memory usage, 1 fewer allocation.
  - Medium: 28.34% faster, 54.55% less memory usage, 1 fewer allocation.
  - Long: 26.90% faster, 73.42% less memory usage, 3 fewer allocations.
  - Long: with Low Max: 19.12% faster, 48.15% less memory usage, 1 fewer
    allocation.

The overall improvements show significant gains in both execution speed
and memory efficiency. For more check eventsreader_bench_test.go.

Changes:
- Preallocated the buffer with a reasonable initial capacity to avoid
  repeated slice resizing.
- Removed TrimLeft call since conversion logic already stops at the
  first nul byte decoded.

2. Explain how to test it

3. Other comments

@geyslan geyslan self-assigned this Jul 18, 2024
@geyslan geyslan marked this pull request as ready for review July 18, 2024 20:04
@geyslan
fix: improve performance of readStringVarFromBuff
28ec953 
Optimize readStringVarFromBuff for better performance and memory usage.

- The optimizations were tested using new benchmarks:
  - Short: 33.26% faster, 42.86% less memory usage, 1 fewer allocation.
  - Medium: 28.34% faster, 54.55% less memory usage, 1 fewer allocation.
  - Long: 26.90% faster, 73.42% less memory usage, 3 fewer allocations.
  - Long: with Low Max: 19.12% faster, 48.15% less memory usage, 1 fewer
    allocation.

The overall improvements show significant gains in both execution speed
and memory efficiency. For more check eventsreader_bench_test.go.

Changes:
- Preallocated the buffer with a reasonable initial capacity without
  initializing data.
- Removed TrimLeft call since conversion logic already stops at the
  first nul byte decoded.
@rscampos
Copy link
Collaborator

Double check the benchmark:

/usr/local/go/bin/go test -benchmem -run=^$ -bench ^(BenchmarkReadStringVarFromBuff_ShortString_warm|BenchmarkReadStringVarFromBuff_ShortString_old|BenchmarkReadStringVarFromBuff_ShortString|BenchmarkReadStringVarFromBuff_MediumString_old|BenchmarkReadStringVarFromBuff_MediumString|BenchmarkReadStringVarFromBuff_LongString_old|BenchmarkReadStringVarFromBuff_LongString|BenchmarkReadStringVarFromBuff_LongStringLowMax_old|BenchmarkReadStringVarFromBuff_LongStringLowMax)$ github.com/aquasecurity/tracee/pkg/bufferdecoder -benchtime=1000000x

=== RUN   BenchmarkReadStringVarFromBuff_ShortString_old
BenchmarkReadStringVarFromBuff_ShortString_old
BenchmarkReadStringVarFromBuff_ShortString_old-4                 1000000                52.02 ns/op           40 B/op          3 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_ShortString
BenchmarkReadStringVarFromBuff_ShortString
BenchmarkReadStringVarFromBuff_ShortString-4                     1000000                27.85 ns/op           16 B/op          2 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_MediumString_old
BenchmarkReadStringVarFromBuff_MediumString_old
BenchmarkReadStringVarFromBuff_MediumString_old-4                1000000                83.48 ns/op           88 B/op          3 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_MediumString
BenchmarkReadStringVarFromBuff_MediumString
BenchmarkReadStringVarFromBuff_MediumString-4                    1000000                49.47 ns/op           40 B/op          2 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_LongString_old
BenchmarkReadStringVarFromBuff_LongString_old
BenchmarkReadStringVarFromBuff_LongString_old-4                  1000000             24929 ns/op           77057 B/op          5 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_LongString
BenchmarkReadStringVarFromBuff_LongString
BenchmarkReadStringVarFromBuff_LongString-4                      1000000             13190 ns/op           20480 B/op          2 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_LongStringLowMax_old
BenchmarkReadStringVarFromBuff_LongStringLowMax_old
BenchmarkReadStringVarFromBuff_LongStringLowMax_old-4            1000000               289.4 ns/op           432 B/op          3 allocs/op
=== RUN   BenchmarkReadStringVarFromBuff_LongStringLowMax
BenchmarkReadStringVarFromBuff_LongStringLowMax
BenchmarkReadStringVarFromBuff_LongStringLowMax-4                1000000               182.8 ns/op           224 B/op          2 allocs/op

The results are aligned with yours:

Short: 46.46% faster, 60.00% less memory usage, 1 fewer allocation.
Medium: 40.74% faster, 54.55% less memory usage, 1 fewer allocation.
Long: 47.09% faster, 73.42% less memory usage, 3 fewer allocations.
Long with Low Max: 36.83% faster, 48.15% less memory usage, 1 fewer allocation.

rscampos
rscampos approved these changes Aug 23, 2024
View reviewed changes
Copy link
Collaborator
@rscampos rscampos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@geyslan geyslan merged commit 773e747 into aquasecurity:main Aug 23, 2024
30 checks passed
@geyslan geyslan deleted the 4081-makezero branch February 19, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NDStrahilevitz NDStrahilevitz NDStrahilevitz left review comments

@rscampos rscampos rscampos approved these changes

@yanivagman yanivagman Awaiting requested review from yanivagman

Assignees

@geyslan geyslan

Labels
area/testing kind/bug milestone/v0.22.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

miss makezero in slice init
3 participants
@geyslan @rscampos @NDStrahilevitz
0