Skip to content
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.

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

Changelog optimization #4242

Merged
merged 2 commits into from
Aug 23, 2024
Merged

Changelog optimization #4242

merged 2 commits into from
Aug 23, 2024

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Aug 10, 2024

1. Explain what the PR does

8cee7a2 perf(changelog): optimize enforceSizeBoundary
3bc61a8 chore(changelog): avoid unnecessary initialization

8cee7a2 perf(changelog): optimize enforceSizeBoundary

These optimizations make the enforceSizeBoundary function more
efficient across various scenarios, leading to overall better
performance while maintaining the same behaviour (*).

Running tool: /home/gg/.goenv/versions/1.22.4/bin/go test -benchmem
-run=^$ -tags ebpf
-bench ^(BenchmarkEnforceSizeBoundaryOld|BenchmarkEnforceSizeBoundary)$
github.com/aquasecurity/tracee/pkg/changelog -benchtime=100000000x

goos: linux
goarch: amd64
pkg: github.com/aquasecurity/tracee/pkg/changelog
cpu: AMD Ryzen 9 7950X 16-Core Processor

| Test Case              | Old (ns/op) | New (ns/op) |   (%)  |
|------------------------|-------------|-------------|--------|
| No change needed       | 1.446       | 1.434       |  0.83% |
| Trim excess duplicates | 30.18       | 21.97       | 27.21% |
| Remove oldest entries  | 27.29       | 21.39       | 21.60% |

Since enforceSizeBoundary is called by setAt, the performance
improvements will be reflected in the latter:

-bench ^(setAtOld|setAt)$

| Test Case          | Old (ns/op) | New (ns/op) |   (%)  |
|--------------------|-------------|-------------|--------|
| All Scenarios      | 1267        | 1163        | 8.21%  |
| Within Limit       | 1770        | 1747        | 1.30%  |

Tests were fixed and added to ensure the correctness of the
implementation. Benchmarks were also added to track the performance
improvements.

(*) The behaviour of the function is the same, but the implementation
fixes a bug that when coalescing duplicate values, the removal of the
oldest entry was not being done correctly. Instead of removing the
oldest entry, the function was removing the newest one.

2. Explain how to test it

3. Other comments

This is part of the proctree effort.

@geyslan

This comment was marked as resolved.

@geyslan geyslan force-pushed the changelog-opt branch 2 times, most recently from 8efa7b3 to c32dcc0 Compare August 12, 2024 12:41
@geyslan geyslan marked this pull request as ready for review August 12, 2024 12:41
These optimizations make the enforceSizeBoundary function more
efficient across various scenarios, leading to overall better
performance while maintaining the same behaviour (*).

Running tool: /home/gg/.goenv/versions/1.22.4/bin/go test -benchmem
-run=^$ -tags ebpf
-bench ^(BenchmarkEnforceSizeBoundaryOld|BenchmarkEnforceSizeBoundary)$
github.com/aquasecurity/tracee/pkg/changelog -benchtime=100000000x

goos: linux
goarch: amd64
pkg: github.com/aquasecurity/tracee/pkg/changelog
cpu: AMD Ryzen 9 7950X 16-Core Processor

| Test Case              | Old (ns/op) | New (ns/op) |   (%)  |
|------------------------|-------------|-------------|--------|
| No change needed       | 1.446       | 1.434       |  0.83% |
| Trim excess duplicates | 30.18       | 21.97       | 27.21% |
| Remove oldest entries  | 27.29       | 21.39       | 21.60% |

Since enforceSizeBoundary is called by setAt, the performance
improvements will be reflected in the latter:

-bench ^(setAtOld|setAt)$

| Test Case          | Old (ns/op) | New (ns/op) |   (%)  |
|--------------------|-------------|-------------|--------|
| All Scenarios      | 1267        | 1163        | 8.21%  |
| Within Limit       | 1770        | 1747        | 1.30%  |

Tests were fixed and added to ensure the correctness of the
implementation. Benchmarks were also added to track the performance
improvements.

(*) The behaviour of the function is the same, but the implementation
fixes a bug that when coalescing duplicate values, the removal of the
oldest entry was not being done correctly. Instead of removing the
oldest entry, the function was removing the newest one.
@geyslan
Copy link
Member Author

geyslan commented Aug 23, 2024

Probably related: #3878

Copy link
Contributor

@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 just double check the setAt because couldn't run it here.

@geyslan
Copy link
Member Author

geyslan commented Aug 23, 2024

LGTM, @geyslan just double check the setAt because couldn't run it here.

Finished (10000000x):

Running tool: /home/gg/.goenv/versions/1.22.4/bin/go test -benchmem -run=^$ -tags ebpf -bench ^Benchmark_setAt$ github.com/aquasecurity/tracee/pkg/changelog -benchtime=10000000x

goos: linux
goarch: amd64
pkg: github.com/aquasecurity/tracee/pkg/changelog
cpu: AMD Ryzen 9 7950X 16-Core Processor            
Benchmark_setAt/All_Scenarios-32         	10000000	      1187 ns/op	    1008 B/op	       7 allocs/op
Benchmark_setAt/Within_Limit-32          	10000000	      1586 ns/op	    2571 B/op	       8 allocs/op

@geyslan geyslan merged commit 4b04800 into aquasecurity:main Aug 23, 2024
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants