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: introduce Bits type and remove Int.Bits and Int.Offset #660

Merged
merged 2 commits into from May 10, 2022

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented May 4, 2022

btf: introduce Bits type

BTF expresses some lengths in bits, others in bytes. So far we don't have a
good convention how to distinguish the two units. Add a Bits type to make
the distinction clearer.

btf: remove Int.Bits and Int.Offset

Unfortunately BTF has two ways to encode bitfields. You can
create a Member and set Offset / BitfieldSize or create an Int that
has the offset and bitfield size baked in. The background for this
can be found in [1].

Remove Int.Bits and Int.Offset in favour of Member.BitfieldSize and
Member.Offset. Also get rid of a bunch of incorrect checks for bitfields,
which we didn't catch due to the weird encoding. This cleans up the API,
but may make our lives worse if we end up having to support writing out
legacy bitfields. Let's hope it doesn't come to that.

The performance of parsing vmlinux hasn't changed dramatically:

    name            old time/op    new time/op    delta
    ParseVmlinux-4    62.1ms ± 1%    61.8ms ± 2%    ~     (p=0.686 n=4+4)

    name            old alloc/op   new alloc/op   delta
    ParseVmlinux-4    44.2MB ± 0%    44.7MB ± 0%  +1.14%  (p=0.029 n=4+4)

    name            old allocs/op  new allocs/op  delta
    ParseVmlinux-4      718k ± 0%      718k ± 0%  +0.00%  (p=0.029 n=4+4)

1: https://github.com/torvalds/linux/commit/9d5f9f701b1891466fb3dbb1806ad97716f95cc3

@lmb lmb added this to the Export package btf milestone May 4, 2022
@lmb lmb requested a review from ti-mo May 9, 2022 08:29
@lmb lmb mentioned this pull request May 9, 2022
lmb added 2 commits May 10, 2022 15:35
BTF expresses some lengths in bits, others in bytes. So far we don't have a
good convention how to distinguish the two units. Add a Bits type to make
the distinction clearer.
Unfortunately BTF has two ways to encode bitfields. You can
create a Member and set Offset / BitfieldSize or create an Int that
has the offset and bitfield size baked in. The background for this
can be found in [1].

Remove Int.Bits and Int.Offset in favour of Member.BitfieldSize and
Member.Offset. Also get rid of a bunch of incorrect checks for bitfields,
which we didn't catch due to the weird encoding. This cleans up the API,
but may make our lives worse if we end up having to support writing out
legacy bitfields. Let's hope it doesn't come to that.

The performance of parsing vmlinux hasn't changed dramatically:

    name            old time/op    new time/op    delta
    ParseVmlinux-4    62.1ms ± 1%    61.8ms ± 2%    ~     (p=0.686 n=4+4)

    name            old alloc/op   new alloc/op   delta
    ParseVmlinux-4    44.2MB ± 0%    44.7MB ± 0%  +1.14%  (p=0.029 n=4+4)

    name            old allocs/op  new allocs/op  delta
    ParseVmlinux-4      718k ± 0%      718k ± 0%  +0.00%  (p=0.029 n=4+4)

1: torvalds/linux@9d5f9f7
@lmb
Copy link
Collaborator Author

lmb commented May 10, 2022

Added TestInflateLegacyBitfield after discussing with Timo.

@lmb lmb merged commit da7057e into cilium:master May 10, 2022
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.

None yet

2 participants