Skip to content

ROX-31831: use hash instead of string#18015

Merged
johannes94 merged 3 commits intojmalsam/optimize-pi-filter-memoryfrom
tj/optimize-pi-filter-memory
Dec 2, 2025
Merged

ROX-31831: use hash instead of string#18015
johannes94 merged 3 commits intojmalsam/optimize-pi-filter-memoryfrom
tj/optimize-pi-filter-memory

Conversation

@janisz
Copy link
Contributor

@janisz janisz commented Dec 2, 2025 •
edited
Loading

@johannes94 I vibe coded a hash optimisation in indicator filter

                                  │  17406       │               new.txt                │
                                  │    sec/op    │    sec/op     vs base                │
BuildIndicatorFilterPerformance-8   384.1m ± 15%   337.0m ± 14%        ~ (p=0.075 n=10)
BuildIndicatorFilterMemory-8         22.40 ±  1%    25.22 ±  6%  +12.58% (p=0.000 n=10)
geomean                              2.933          2.915         -0.62%

                                  │  17406     │               new.txt                │
                                  │     B/op     │     B/op      vs base                │
BuildIndicatorFilterPerformance-8   163.9Mi ± 0%   144.5Mi ± 0%  -11.82% (p=0.000 n=10)
BuildIndicatorFilterMemory-8        769.2Mi ± 0%   750.1Mi ± 0%   -2.48% (p=0.000 n=10)
geomean                             355.1Mi        329.2Mi        -7.27%

                                  │ 17406       │               new.txt               │
                                  │  allocs/op  │  allocs/op   vs base                │
BuildIndicatorFilterPerformance-8   1.902M ± 0%   1.602M ± 0%  -15.77% (p=0.000 n=10)
BuildIndicatorFilterMemory-8        14.69M ± 0%   14.39M ± 0%   -2.04% (p=0.000 n=10)
geomean                             5.287M        4.802M        -9.16%

                             │ 17406       │             new-unit.txt             │
                             │    sec/op    │    sec/op     vs base                │
Add-8                          551.0n ±  3%   530.9n ± 10%        ~ (p=0.123 n=10)
AddMemory-8                    516.7n ±  8%   480.4n ± 15%        ~ (p=0.075 n=10)
BuildIndicatorFilterMemory-8   26.35m ± 17%   21.68m ±  5%  -17.74% (p=0.000 n=10)
geomean                        19.58µ         17.68µ         -9.67%

                             │ 17406      │             new-unit.txt             │
                             │     B/op     │     B/op      vs base                │
Add-8                           100.00 ± 0%     64.00 ± 0%  -36.00% (p=0.000 n=10)
AddMemory-8                      76.00 ± 0%     48.00 ± 0%  -36.84% (p=0.000 n=10)
BuildIndicatorFilterMemory-8   17.22Mi ± 0%   14.11Mi ± 0%  -18.05% (p=0.000 n=10)
geomean                        5.037Ki        3.485Ki       -30.81%

                             │ 17406        │            new-unit.txt             │
                             │  allocs/op   │  allocs/op   vs base                │
Add-8                            5.000 ± 0%    1.000 ± 0%  -80.00% (p=0.000 n=10)
AddMemory-8                      5.000 ± 0%    1.000 ± 0%  -80.00% (p=0.000 n=10)
BuildIndicatorFilterMemory-8    247.6k ± 0%   197.6k ± 0%  -20.19% (p=0.000 n=10)
geomean                          183.6         58.25       -68.28%

@janisz
perf: use hash instead of string
8d34245 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz janisz requested a review from johannes94 December 2, 2025 10:50
@janisz
add benchmark
3fe532a 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Dec 2, 2025 •
edited
Loading

Images are ready for the commit at d0dc554.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.10.x-476-gd0dc55422c.

@codecov
Copy link

codecov bot commented Dec 2, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.47%. Comparing base (cca864c) to head (d0dc554).
⚠️ Report is 1 commits behind head on jmalsam/optimize-pi-filter-memory.

Files with missing lines Patch % Lines
pkg/process/filter/filter.go 89.65% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@                        Coverage Diff                         @@
##           jmalsam/optimize-pi-filter-memory   #18015   +/-   ##
==================================================================
  Coverage                              49.47%   49.47%           
==================================================================
  Files                                   2699     2699           
  Lines                                 198168   198187   +19     
==================================================================
+ Hits                                   98050    98060   +10     
- Misses                                 92519    92524    +5     
- Partials                                7599     7603    +4
Flag Coverage Δ
go-unit-tests 49.47% <89.65%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@janisz
cleanup
d0dc554 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
johannes94
johannes94 approved these changes Dec 2, 2025
View reviewed changes
Copy link
Contributor

@johannes94 johannes94 left a comment

Choose a reason for hiding this comment

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

@janisz I think I misunderstood what you meant with your comment on the initial PR. Thanks for implementing this.

The concern I talked about previously with the Jaccard Similarity is pruning logic for PIs it does not apply here.

@johannes94 johannes94 merged commit c5cabb8 into jmalsam/optimize-pi-filter-memory Dec 2, 2025
81 checks passed
@johannes94 johannes94 deleted the tj/optimize-pi-filter-memory branch December 2, 2025 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johannes94 johannes94 johannes94 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@janisz @rhacs-bot @johannes94