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

Make sure that the key argument on Get doesn't need to be allocated on heap #12

Open
cipriancraciun opened this issue Dec 26, 2021 · 0 comments

Comments

@cipriancraciun
Copy link

Currently, when one calls cdb.Get(key), if the caller has the key on stack, like key := [4]byte {0,1,2,3}, the Go compiler decides that the value "escapes" and thus it must be allocated on the heap. (The reason being that cdb.hash uses that key value, and an arbitrary implementation might decide to keep a slice of the key.)

This can be spotted with the following simple test:

func BenchmarkStackEscapeYes(b *testing.B) {
    db, err := cdb.OpenMmap("./test/test.cdb")
    require.NoError(b, err)
    require.NotNil(b, db)

    for i := 0; i < b.N; i++ {
        keyOnStack := [2]byte {'X', byte (i)}
        keySlice := keyOnStack[:]
        db.Get(keySlice)
    }
}

If comipilled as go test -gcflags -m one can see the message ./noescape_test.go:32:3: moved to heap: keyOnStack.

The following two patches solve this issue:

The performance improvements are about ~10ns per call, which if also using the mmap patch sent earlier represents 50% of the total runtime.

BenchmarkStackEscapeNo-4        1000000000               9.368 ns/op           0 B/op          0 allocs/op
BenchmarkStackEscapeYes-4       1000000000              23.12 ns/op            2 B/op          1 allocs/op
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

No branches or pull requests

1 participant