Skip to content

Comments

collection: share kernel BTF when loading a collection#1778

Merged
lmb merged 1 commit intocilium:mainfrom
lmb:collection-share-btf
May 21, 2025
Merged

collection: share kernel BTF when loading a collection#1778
lmb merged 1 commit intocilium:mainfrom
lmb:collection-share-btf

Conversation

@lmb
Copy link
Contributor

@lmb lmb commented May 7, 2025 •
edited
Loading 

We currently create a new copy of kernel BTF when loading each program in a 
collection, which is quite expensive.

Introduce a new type btf.Cache which allows re-using kernel BTF as-needed. 
As an optimisation, the Cache is opportunistically populated from the global 
cache on creation.

    core: 1
   goos: linux
   goarch: amd64
   pkg: github.com/cilium/ebpf
   cpu: 13th Gen Intel(R) Core(TM) i7-1365U
                       │   base.txt   │             cache.txt              │
                       │    sec/op    │   sec/op     vs base               │
   NewCollectionManyProgs   347.84m ± 1%   14.54m ± 2%  -95.82% (p=0.002
n=6)

                        │    base.txt    │              cache.txt           
  │
                       │      B/op      │     B/op      vs base             
 │
   NewCollectionManyProgs   108.056Mi ± 0%   3.841Mi ± 0%  -96.44% (p=0.002
n=6)

                        │   base.txt    │             cache.txt             
│
                       │   allocs/op   │  allocs/op   vs base              
│
   NewCollectionManyProgs   1026.83k ± 0%   38.04k ± 0%  -96.30% (p=0.002
n=6)

Signed-off-by: Lorenz Bauer <[email protected]>

@lmb
Copy link
Contributor Author

lmb commented May 7, 2025

@patrickpichler could you give this branch a try and let me know what you think?

@lmb lmb mentioned this pull request May 7, 2025
Closed
@lmb lmb force-pushed the collection-share-btf branch from 0001b34 to cbaa060 Compare May 20, 2025 16:32
@lmb lmb changed the title collection: share kernel BTF when loading programs from single collec… collection: share kernel BTF when loading a collection May 20, 2025
@lmb lmb marked this pull request as ready for review May 20, 2025 16:33
@lmb lmb requested review from a team and dylandreimerink as code owners May 20, 2025 16:33
@patrickpichler
Copy link

patrickpichler commented May 21, 2025

Sorry for the super late response. I did run the benchmark again and here are the results:
Without your change
BenchmarkNewCollectionManyProgs-4 15 75442629 ns/op 74477811 B/op 809763 allocs/op

With your change
BenchmarkNewCollectionManyProgs-4 272 4193884 ns/op 2734829 B/op 30806 allocs/op

With the optimization I did in the draft PR (#1690)
BenchmarkNewCollectionManyProgs-4 184 5807742 ns/op 4134444 B/op 17325 allocs/op

When performing a single run, the total number of allocated memory according to pprof went down from ~660MB to ~26MB, so this is definitely a win.

I guess there are a few more ways on how to reduce the number of allocations, but for my use case, this should be good enough.

Thanks a lot!

@lmb lmb force-pushed the collection-share-btf branch 2 times, most recently from 37ff654 to feb0a51 Compare May 21, 2025 09:15
@lmb
collection: share kernel BTF when loading a collection
7ed1590 
We currently create a new copy of kernel BTF when loading each program in a
collection, which is quite expensive.

Introduce a new type btf.Cache which allows re-using kernel BTF as-needed.
As an optimisation, the Cache is opportunistically populated from the global
cache on creation.

    core: 1
    goos: linux
    goarch: amd64
    pkg: github.com/cilium/ebpf
    cpu: 13th Gen Intel(R) Core(TM) i7-1365U
                        │   base.txt   │             cache.txt              │
                        │    sec/op    │   sec/op     vs base               │
    NewCollectionManyProgs   347.84m ± 1%   14.54m ± 2%  -95.82% (p=0.002 n=6)

                        │    base.txt    │              cache.txt              │
                        │      B/op      │     B/op      vs base               │
    NewCollectionManyProgs   108.056Mi ± 0%   3.841Mi ± 0%  -96.44% (p=0.002 n=6)

                        │   base.txt    │             cache.txt              │
                        │   allocs/op   │  allocs/op   vs base               │
    NewCollectionManyProgs   1026.83k ± 0%   38.04k ± 0%  -96.30% (p=0.002 n=6)

Signed-off-by: Lorenz Bauer <[email protected]>
@lmb lmb force-pushed the collection-share-btf branch from feb0a51 to 7ed1590 Compare May 21, 2025 09:48
dylandreimerink
dylandreimerink approved these changes May 21, 2025
View reviewed changes
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

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

This seems sane to me 🚀

@lmb lmb merged commit dd4d949 into cilium:main May 21, 2025
17 of 18 checks passed
@lmb lmb deleted the collection-share-btf branch May 21, 2025 10:19
@mauriciovasquezbernal mauriciovasquezbernal mentioned this pull request May 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dylandreimerink dylandreimerink dylandreimerink 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

@lmb @patrickpichler @dylandreimerink