map: zero-allocation operations for common types #1062

lmb · 2023-06-14T15:10:27Z

map: zero-allocation operations for common types

Map keys and values are currently marshaled into []byte by souped up
versions of binary.Write and binary.Read. This allows users to be blissfully
unaware of compiler inserted padding on the Go side. This is wasteful in
case the Go in-memory representation matches what the kernel expects because
we need additional allocations.

Refactor syscall marshaling into a new package sysenc which encapsulates the
logic we need to determine whether a Go type is safe for zero-allocation /
zero-copy marshaling. The type must be a pointer to or a slice of:

* A primitive type like uint32, ... or
* An array of valid types or
* A struct made up of valid types without any compiler
 inserted padding between fields

Per-CPU maps don't support zero-allocation operations for now, but the new
code already makes things a little bit cheaper.

Structs with trailing padding also don't benefit from the optimization for
now. Consider

    type padded struct { A uint32; B uint16 }

Allowing such a type creates an edge case: make([]padding, 1) uses
zero-allocation marshaling while make([]padding, 2) doesn't, due to interior
padding. It's simpler to skip such types for now.

    goos: linux
   goarch: amd64
   pkg: github.com/cilium/ebpf
   cpu: 12th Gen Intel(R) Core(TM) i7-1260P
                                        │ unsafe.txt  │
                                        │   sec/op    │
   Marshaling/ValueUnmarshalReflect-16    356.1n ± 2%
   Marshaling/KeyMarshalReflect-16        368.6n ± 1%
   Marshaling/ValueBinaryUnmarshaler-16   378.6n ± 2%
   Marshaling/KeyBinaryMarshaler-16       356.2n ± 1%
   Marshaling/KeyValueUnsafe-16           328.0n ± 2%
   PerCPUMarshalling/reflection-16        1.232µ ± 1%

                                         │  unsafe.txt  │
                                        │     B/op     │
   Marshaling/ValueUnmarshalReflect-16    0.000 ± 0%
   Marshaling/KeyMarshalReflect-16        0.000 ± 0%
   Marshaling/ValueBinaryUnmarshaler-16   24.00 ± 0%
   Marshaling/KeyBinaryMarshaler-16       8.000 ± 0%
   Marshaling/KeyValueUnsafe-16           0.000 ± 0%
   PerCPUMarshalling/reflection-16        280.0 ± 0%

                                         │  unsafe.txt  │
                                        │  allocs/op   │
   Marshaling/ValueUnmarshalReflect-16    0.000 ± 0%
   Marshaling/KeyMarshalReflect-16        0.000 ± 0%
   Marshaling/ValueBinaryUnmarshaler-16   1.000 ± 0%
   Marshaling/KeyBinaryMarshaler-16       1.000 ± 0%
   Marshaling/KeyValueUnsafe-16           0.000 ± 0%
   PerCPUMarshalling/reflection-16        3.000 ± 0%

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

internal/sysenc/marshal.go

lmb · 2023-06-28T09:05:41Z

@mejedi I'll be doing a code walk of this PR tomorrow 9:30 UTC+1, let me know if you'd like to join.

mejedi · 2023-06-29T03:06:48Z

@lmb

@mejedi I'll be doing a code walk of this PR tomorrow 9:30 UTC+1, let me know if you'd like to join.

Thank you for the invitation! I can’t join, I’m afraid. I’m pretty much preoccupied with moving apartments this week :(

internal/sysenc/marshal.go

internal/sysenc/layout.go

lmb · 2023-07-04T16:54:50Z

I removed the custom layout code after discussing it with Robin and Timo.

ti-mo

Sorry for leaving this unattended for so long. I have a few questions and left a few comments. Thanks! 🙏

marshalers.go

internal/sysenc/buffer.go

internal/sysenc/layout.go

internal/sysenc/marshal.go

marshalers.go

internal/sysenc/layout_test.go

Map keys and values are currently marshaled into []byte by souped up versions of binary.Write and binary.Read. This allows users to be blissfully unaware of compiler inserted padding on the Go side. This is wasteful in case the Go in-memory representation matches what the kernel expects because we need additional allocations. Refactor syscall marshaling into a new package sysenc which encapsulates the logic we need to determine whether a Go type is safe for zero-allocation / zero-copy marshaling. The type must be a pointer to or a slice of: * A primitive type like uint32, ... or * An array of valid types or * A struct made up of valid types without any compiler inserted padding between fields Per-CPU maps don't support zero-allocation operations for now, but the new code already makes things a little bit cheaper. Structs with trailing padding also don't benefit from the optimization for now. Consider type padded struct { A uint32; B uint16 } Allowing such a type creates an edge case: make([]padding, 1) uses zero-allocation marshaling while make([]padding, 2) doesn't, due to interior padding. It's simpler to skip such types for now. goos: linux goarch: amd64 pkg: github.com/cilium/ebpf cpu: 12th Gen Intel(R) Core(TM) i7-1260P │ unsafe.txt │ │ sec/op │ Marshaling/ValueUnmarshalReflect-16 356.1n ± 2% Marshaling/KeyMarshalReflect-16 368.6n ± 1% Marshaling/ValueBinaryUnmarshaler-16 378.6n ± 2% Marshaling/KeyBinaryMarshaler-16 356.2n ± 1% Marshaling/KeyValueUnsafe-16 328.0n ± 2% PerCPUMarshalling/reflection-16 1.232µ ± 1% │ unsafe.txt │ │ B/op │ Marshaling/ValueUnmarshalReflect-16 0.000 ± 0% Marshaling/KeyMarshalReflect-16 0.000 ± 0% Marshaling/ValueBinaryUnmarshaler-16 24.00 ± 0% Marshaling/KeyBinaryMarshaler-16 8.000 ± 0% Marshaling/KeyValueUnsafe-16 0.000 ± 0% PerCPUMarshalling/reflection-16 280.0 ± 0% │ unsafe.txt │ │ allocs/op │ Marshaling/ValueUnmarshalReflect-16 0.000 ± 0% Marshaling/KeyMarshalReflect-16 0.000 ± 0% Marshaling/ValueBinaryUnmarshaler-16 1.000 ± 0% Marshaling/KeyBinaryMarshaler-16 1.000 ± 0% Marshaling/KeyValueUnsafe-16 0.000 ± 0% PerCPUMarshalling/reflection-16 3.000 ± 0% Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

lmb · 2023-09-14T15:32:31Z

@ti-mo this is good to go.

lmb commented Jun 14, 2023

View reviewed changes

internal/sysenc/marshal.go Outdated Show resolved Hide resolved

lmb force-pushed the unsafe-marshal branch 3 times, most recently from ec9482c to f81ea58 Compare June 19, 2023 09:46

mejedi reviewed Jun 19, 2023

View reviewed changes

internal/sysenc/marshal.go Show resolved Hide resolved

lmb force-pushed the unsafe-marshal branch 4 times, most recently from be6a923 to f19814b Compare June 26, 2023 15:05