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

schema/gen/go: avoid Maybe pointers for small types #160

Merged
merged 1 commit into from
Apr 7, 2021
Merged

schema/gen/go: avoid Maybe pointers for small types #160

merged 1 commit into from
Apr 7, 2021

Commits on Apr 7, 2021

  1. schema/gen/go: avoid Maybe pointers for small types

    If we know that a schema type can be represented in Go with a small
    amount of bytes, using a pointer to store its "maybe" is rarely a good
    idea. For example, an optional string only weighs twice as much as a
    pointer, so a pointer adds overhead and will barely ever save any
    memory.
    
    Add a function to work out the byte size of a schema.TypeKind, relying
    on reflection and the basicnode package. Debug prints are also present
    if one wants to double-check the numbers. As of today, they are:
    
    	sizeOf(small): 32 (4x pointer size)
    	sizeOf(Bool): 1
    	sizeOf(Int): 8
    	sizeOf(Float): 8
    	sizeOf(String): 16
    	sizeOf(Bytes): 24
    	sizeOf(List): 24
    	sizeOf(Map): 32
    	sizeOf(Link): 16
    
    Below is the result on go-merkledag's BenchmarkRoundtrip after
    re-generating go-codec-dagpb with this change. Note that the dag-pb
    schema contains multiple optional fields, such as strings.
    
    	name         old time/op    new time/op    delta
    	Roundtrip-8    4.24µs ± 3%    3.78µs ± 0%  -10.87%  (p=0.004 n=6+5)
    
    	name         old alloc/op   new alloc/op   delta
    	Roundtrip-8    6.38kB ± 0%    6.24kB ± 0%   -2.26%  (p=0.002 n=6+6)
    
    	name         old allocs/op  new allocs/op  delta
    	Roundtrip-8       103 ± 0%        61 ± 0%  -40.78%  (p=0.002 n=6+6)
    
    Schema typekinds which don't directly map to basicnode prototypes, such
    as structs and unions, are left as a TODO for now.
    
    I did not do any measurements to arrive at the magic number of 4x, which
    is documented in the code. We might well increase it in the future, with
    more careful benchmarking. For now, it seems like a conservative starting
    point that should cover all basic types.
    
    Finally, re-generate within this repo.
    mvdan committed Apr 7, 2021
    Configuration menu
    Copy the full SHA
    fc8d360 View commit details
    Browse the repository at this point in the history