Skip to content

Commit

Permalink
Benchmarks of various Cid types as map keys.
Browse files Browse the repository at this point in the history
And writeup on same.

tl;dr interfaces are not cheap if you're already at the scale where you
started caring about whether or not you have pointers.
  • Loading branch information
warpfork committed Aug 24, 2018
1 parent 5a6d4bd commit 2cf56e3
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 2 deletions.
20 changes: 20 additions & 0 deletions _rsrch/cidiface/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,26 @@ If these traversals are *not* a significant timesink, we might be wiser
to keep to Option B, because keeping a struct full of offsets will add several
words of memory usage per CID, and we keep a *lot* of CIDs.

### interfaces cause boxing which is a significant performance cost

See `BenchmarkCidMap_CidStr` and friends.

Long story short: using interfaces *anywhere* will cause the compiler to
implicitly generate boxing and unboxing code (e.g. `runtime.convT2E`);
this is both another function call, and more concerningly, results in
large numbers of unbatchable memory allocations.

Numbers without context are dangerous, but if you need one: 33%.
It's a big deal.

This means attempts to "use interfaces, but switch to concrete impls when
performance is important" are a red herring: it doesn't work that way.

This is not a general inditement against using interfaces -- but
if a situation is at the scale where it's become important to mind whether
or not pointers are a performance impact, then that situation also
is one where you have to think twice before using interfaces.

### one way or another: let's get rid of that star

We should switch completely to handling `Cid` and remove `*Cid` completely.
Expand Down
71 changes: 71 additions & 0 deletions _rsrch/cidiface/cidBoxingBench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package cid

import (
"testing"
)

// BenchmarkCidMap_CidStr estimates how fast it is to insert primitives into a map
// keyed by CidStr (concretely).
//
// We do 100 insertions per benchmark run to make sure the map initialization
// doesn't dominate the results.
//
// Sample results on linux amd64 go1.11beta:
//
// BenchmarkCidMap_CidStr-8 100000 16317 ns/op
// BenchmarkCidMap_CidIface-8 100000 20516 ns/op
//
// With benchmem on:
//
// BenchmarkCidMap_CidStr-8 100000 15579 ns/op 11223 B/op 207 allocs/op
// BenchmarkCidMap_CidIface-8 100000 19500 ns/op 12824 B/op 307 allocs/op
// BenchmarkCidMap_StrPlusHax-8 200000 10451 ns/op 7589 B/op 202 allocs/op
//
// We can see here that the impact of interface boxing is significant:
// it increases the time taken to do the inserts to 133%, largely because
// the implied `runtime.convT2E` calls cause another malloc each.
//
// There are also significant allocations in both cases because
// A) we cannot create a multihash without allocations since they are []byte;
// B) the map has to be grown several times;
// C) something I haven't quite put my finger on yet.
// Ideally we'd drive those down further as well.
//
// Pre-allocating the map reduces allocs by a very small percentage by *count*,
// but reduces the time taken by 66% overall (presumably because when a map
// re-arranges itself, it involves more or less an O(n) copy of the content
// in addition to the alloc itself). This isn't topical to the question of
// whether or not interfaces are a good idea; just for contextualizing.
//
func BenchmarkCidMap_CidStr(b *testing.B) {
for i := 0; i < b.N; i++ {
mp := map[CidStr]int{}
for x := 0; x < 100; x++ {
mp[NewCidStr(0, uint64(x), []byte{})] = x
}
}
}

// BenchmarkCidMap_CidIface is in the family of BenchmarkCidMap_CidStr:
// it is identical except the map key type is declared as an interface
// (which forces all insertions to be boxed, changing performance).
func BenchmarkCidMap_CidIface(b *testing.B) {
for i := 0; i < b.N; i++ {
mp := map[Cid]int{}
for x := 0; x < 100; x++ {
mp[NewCidStr(0, uint64(x), []byte{})] = x
}
}
}

// BenchmarkCidMap_CidStrAvoidMapGrowth is in the family of BenchmarkCidMap_CidStr:
// it is identical except the map is created with a size hint that removes
// some allocations (5, in practice, apparently).
func BenchmarkCidMap_CidStrAvoidMapGrowth(b *testing.B) {
for i := 0; i < b.N; i++ {
mp := make(map[CidStr]int, 100)
for x := 0; x < 100; x++ {
mp[NewCidStr(0, uint64(x), []byte{})] = x
}
}
}
4 changes: 2 additions & 2 deletions _rsrch/cidiface/cidString.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (c CidStr) Prefix() Prefix {
// parsers & validators & factories
//==================================

func newCidStr(version uint64, codecType uint64, mhash mh.Multihash) CidStr {
func NewCidStr(version uint64, codecType uint64, mhash mh.Multihash) CidStr {
hashlen := len(mhash)
// two 8 bytes (max) numbers plus hash
buf := make([]byte, 2*binary.MaxVarintLen64+hashlen)
Expand Down Expand Up @@ -133,7 +133,7 @@ func CidStrParse(data []byte) (CidStr, error) {
if err != nil {
return EmptyCidStr, err
}
return newCidStr(0, DagProtobuf, h), nil
return NewCidStr(0, DagProtobuf, h), nil
}

vers, n := binary.Uvarint(data)
Expand Down

0 comments on commit 2cf56e3

Please sign in to comment.