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

btf: don't copy types during CO-RE relocation #1084

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Jul 3, 2023

btf: add CO-RE benchmark

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: don't copy types during CO-RE relocation

During CO-RE relocation we need to ignore qualifiers and Typedefs. So far
we've done this by creating a full copy of each target type with those types
stripped out.It turns out that this is really expensive especially for large
types like struct sk_buff.

Stop copying types during CO-RE relocation and instead use the existing
UnderlyingType() and a new helper as() to skip when appropriate. As a rule
of thumb, anytime we do a type assertion during CO-RE relocation we should
use one of these helpers as appropriate.

Embarrassingly this leads to a speedup of 99.99% for large types:

    goos: linux
   goarch: amd64
   pkg: github.com/cilium/ebpf/btf
   cpu: 12th Gen Intel(R) Core(TM) i7-1260P
                               │     base.txt     │               opt.txt   
            │
                               │      sec/op      │    sec/op     vs base   
            │
   CORESkBuff/byte_off-16         2987010.0n ±  2%   311.9n ±  1%   -99.99%
(p=0.002 n=6)
   CORESkBuff/byte_sz-16          3016966.0n ±  0%   348.2n ±  1%   -99.99%
(p=0.002 n=6)
   CORESkBuff/field_exists-16     3036697.5n ±  2%   320.9n ±  2%   -99.99%
(p=0.002 n=6)
   CORESkBuff/signed-16           3133408.5n ±  3%   211.9n ±  1%   -99.99%
(p=0.002 n=6)
   CORESkBuff/lshift_u64-16       3129531.5n ±  5%   358.9n ±  4%   -99.99%
(p=0.002 n=6)
   CORESkBuff/rshift_u64-16       3131090.5n ±  6%   346.9n ±  1%   -99.99%
(p=0.002 n=6)
   CORESkBuff/local_type_id-16        23.83n ±  2%   23.12n ±  2%    -2.98%
(p=0.002 n=6)
   CORESkBuff/target_type_id-16   3660381.0n ± 48%   266.7n ±  4%   -99.99%
(p=0.002 n=6)
   CORESkBuff/type_exists-16      6158062.0n ± 18%   256.6n ±  6%  -100.00%
(p=0.002 n=6)
   CORESkBuff/type_size-16        4724512.0n ±  6%   282.9n ±  1%   -99.99%
(p=0.002 n=6)
   CORESkBuff/enumval_exists-16       703.0n ±  7%   271.2n ± 12%   -61.42%
(p=0.002 n=6)
   CORESkBuff/enumval_value-16        749.0n ±  9%   335.8n ± 18%   -55.16%
(p=0.002 n=6)
   geomean                            319.3µ         240.5n         -99.92%

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

Updates #1081

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
@lmb lmb requested a review from rgo3 July 3, 2023 10:40
@lmb
Copy link
Collaborator Author

lmb commented Jul 3, 2023

FYI @brycekahle

During CO-RE relocation we need to ignore qualifiers and Typedefs.
So far we've done this by creating a full copy of each target type
with those types stripped out.It turns out that this is really
expensive especially for large types like struct sk_buff.

Stop copying types during CO-RE relocation and instead use the
existing UnderlyingType() and a new helper as() to skip
when appropriate. As a rule of thumb, anytime we do a type
assertion during CO-RE relocation we should use one of these
helpers as appropriate.

Embarrassingly this leads to a speedup of 99.99% for large
types:

    goos: linux
    goarch: amd64
    pkg: github.com/cilium/ebpf/btf
    cpu: 12th Gen Intel(R) Core(TM) i7-1260P
                                │     base.txt     │               opt.txt                │
                                │      sec/op      │    sec/op     vs base                │
    CORESkBuff/byte_off-16         2987010.0n ±  2%   311.9n ±  1%   -99.99% (p=0.002 n=6)
    CORESkBuff/byte_sz-16          3016966.0n ±  0%   348.2n ±  1%   -99.99% (p=0.002 n=6)
    CORESkBuff/field_exists-16     3036697.5n ±  2%   320.9n ±  2%   -99.99% (p=0.002 n=6)
    CORESkBuff/signed-16           3133408.5n ±  3%   211.9n ±  1%   -99.99% (p=0.002 n=6)
    CORESkBuff/lshift_u64-16       3129531.5n ±  5%   358.9n ±  4%   -99.99% (p=0.002 n=6)
    CORESkBuff/rshift_u64-16       3131090.5n ±  6%   346.9n ±  1%   -99.99% (p=0.002 n=6)
    CORESkBuff/local_type_id-16        23.83n ±  2%   23.12n ±  2%    -2.98% (p=0.002 n=6)
    CORESkBuff/target_type_id-16   3660381.0n ± 48%   266.7n ±  4%   -99.99% (p=0.002 n=6)
    CORESkBuff/type_exists-16      6158062.0n ± 18%   256.6n ±  6%  -100.00% (p=0.002 n=6)
    CORESkBuff/type_size-16        4724512.0n ±  6%   282.9n ±  1%   -99.99% (p=0.002 n=6)
    CORESkBuff/enumval_exists-16       703.0n ±  7%   271.2n ± 12%   -61.42% (p=0.002 n=6)
    CORESkBuff/enumval_value-16        749.0n ±  9%   335.8n ± 18%   -55.16% (p=0.002 n=6)
    geomean                            319.3µ         240.5n         -99.92%

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
@lmb lmb merged commit 8fa4c90 into cilium:master Jul 4, 2023
1 check passed
@lmb lmb deleted the core-no-copy branch July 4, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants