Skip to content

Use new set.Adaptive type in named port index #10027

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

fasaxc
Copy link
Member

@fasaxc fasaxc commented Mar 21, 2025

Description

Add an "adaptive" implementation of set.Set that uses a different storage approach depending on number of items.

  • 0 items: store a nil pointer.
  • 1 item: store pointer to the item
  • 2-16 items: store pointer to array of that length rounded up to power of 2
  • 17+: store pointer to a proper map.

It is faster and uses a fraction of the RAM for 0-16 items, then its performance becomes similar to the normal set.Typed (with slight overhead).

Use the map in the named port index, where we have many sets that are generally expected to be very small.

It knocks 20% off the BenchmarkSnapshot200Local250kTotal10000TagPols benchmark, which has a very large number of small sets. It gives almost no improvement on the others, but those are dominated by having large numbers of active policies and very few labels.

Before:

goos: linux
goarch: amd64
pkg: github.com/projectcalico/calico/felix/calc
cpu: Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
BenchmarkSnapshot200Local250kTotal10000TagPols-12                     	       1	3245931822 ns/op	       307.9 HeapAllocMB	        29.00 IPSets	     10432 Msgs	     10000 Policies	         3.246 s	970140472 B/op	10684941 allocs/op
BenchmarkSnapshot200Local10kTotal1000NetsetPols-12                    	       2	 516745681 ns/op	        78.60 HeapAllocMB	      1000 IPSets	      2403 Msgs	      1000 Policies	         0.5117 s	245642908 B/op	 1909899 allocs/op
BenchmarkSnapshot200Local10kTotal10000NetsetPols-12                   	       1	5866074645 ns/op	       604.9 HeapAllocMB	     10000 IPSets	     20403 Msgs	     10000 Policies	         5.866 s	2525107568 B/op	17306981 allocs/op

After:

goos: linux
goarch: amd64
pkg: github.com/projectcalico/calico/felix/calc
cpu: Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
BenchmarkSnapshot200Local250kTotal10000TagPols-12                     	       1	3230500160 ns/op	       249.8 HeapAllocMB	        29.00 IPSets	     10432 Msgs	     10000 Policies	         3.230 s	929163456 B/op	11895089 allocs/op
BenchmarkSnapshot200Local10kTotal1000NetsetPols-12                    	       2	 510494161 ns/op	        77.94 HeapAllocMB	      1000 IPSets	      2403 Msgs	      1000 Policies	         0.5055 s	245306080 B/op	 1962315 allocs/op
BenchmarkSnapshot200Local10kTotal10000NetsetPols-12                   	       1	5766052793 ns/op	       598.9 HeapAllocMB	     10000 IPSets	     20403 Msgs	     10000 Policies	         5.766 s	2514418280 B/op	17287133 allocs/op

Also included: store sets as their concrete type in the places where we store a lot of them; saves a few MB.

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

Reduce memory usage per endpoint in the selector index.

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@fasaxc fasaxc requested a review from a team as a code owner March 21, 2025 11:46
@marvin-tigera marvin-tigera added this to the Calico v3.30.0 milestone Mar 21, 2025
@marvin-tigera marvin-tigera added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Mar 21, 2025
@fasaxc fasaxc force-pushed the adaptive-set branch 2 times, most recently from 0a336da to 54f7f1a Compare March 21, 2025 13:58
@fasaxc fasaxc changed the title Use adaptive set in named port index [WIP] Use adaptive set in named port index Mar 24, 2025
@fasaxc fasaxc marked this pull request as draft April 30, 2025 09:40
@fasaxc fasaxc changed the title [WIP] Use adaptive set in named port index Use adaptive set in named port index Apr 30, 2025
@fasaxc fasaxc added docs-not-required Docs not required for this change and removed docs-pr-required Change is not yet documented labels Apr 30, 2025
@fasaxc fasaxc marked this pull request as ready for review April 30, 2025 15:31
fasaxc added 4 commits May 7, 2025 11:54
Stores its contents differently depending on the number of items.

- 0 items -> size=0 ptr=nil
- 1 item  -> size=1 ptr=<pointer to item>
- 2-16 items -> size=n ptr=<pointer to array of items>
- >16 -> size=-1 ptr=<pointer to map[T]struct{}>
Should reduce memory usage significantly.
Saves 8 bytes per instance, which can add up in some places.
@fasaxc fasaxc changed the title Use adaptive set in named port index Use new set.Adaptive type in named port index May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required Docs not required for this change release-note-required Change has user-facing impact (no matter how small)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants