Skip to content

Commit

Permalink
binary/reader: Skip fixed witdth collections faster
Browse files Browse the repository at this point in the history
When skipping over collections in binary.Reader, we currently do:

    for i := 0; i < length; i++ {
        skip()
    }

This is suboptimal for cases where the number of bytes skipped by
`skip()` will be the same for each call because we can just skip
`length * width` bytes.

Re-use logic from the streaming reader that already does this when
possible.

```
name                                                  old time/op    new time/op    delta
RoundTrip/PrimitiveOptionalStruct/Decode-4              3.03µs ±10%    2.91µs ± 2%   -3.92%  (p=0.013 n=10+9)
RoundTrip/PrimitiveOptionalStruct/Streaming_Decode-4    1.90µs ± 2%    1.88µs ± 3%   -1.01%  (p=0.033 n=10+9)
RoundTrip/Graph/Decode-4                                9.88µs ± 2%    9.93µs ± 4%     ~     (p=0.714 n=8+10)
RoundTrip/Graph/Streaming_Decode-4                      2.72µs ± 2%    2.70µs ± 5%     ~     (p=0.258 n=9+9)
RoundTrip/ContainersOfContainers/Decode-4                133µs ± 9%      69µs ± 1%  -48.47%  (p=0.000 n=10+8)
RoundTrip/ContainersOfContainers/Streaming_Decode-4     31.0µs ± 5%    30.4µs ± 2%   -2.01%  (p=0.043 n=9+10)

name                                                  old alloc/op   new alloc/op   delta
RoundTrip/PrimitiveOptionalStruct/Decode-4              1.40kB ± 0%    1.40kB ± 0%     ~     (all equal)
RoundTrip/PrimitiveOptionalStruct/Streaming_Decode-4     56.0B ± 0%     56.0B ± 0%     ~     (all equal)
RoundTrip/Graph/Decode-4                                3.52kB ± 0%    3.52kB ± 0%     ~     (all equal)
RoundTrip/Graph/Streaming_Decode-4                        168B ± 0%      168B ± 0%     ~     (all equal)
RoundTrip/ContainersOfContainers/Decode-4               29.3kB ± 0%    15.7kB ± 0%  -46.36%  (p=0.000 n=10+9)
RoundTrip/ContainersOfContainers/Streaming_Decode-4     10.1kB ± 0%    10.1kB ± 0%     ~     (all equal)

name                                                  old allocs/op  new allocs/op  delta
RoundTrip/PrimitiveOptionalStruct/Decode-4                14.0 ± 0%      14.0 ± 0%     ~     (all equal)
RoundTrip/PrimitiveOptionalStruct/Streaming_Decode-4      10.0 ± 0%      10.0 ± 0%     ~     (all equal)
RoundTrip/Graph/Decode-4                                  63.0 ± 0%      63.0 ± 0%     ~     (all equal)
RoundTrip/Graph/Streaming_Decode-4                        10.0 ± 0%      10.0 ± 0%     ~     (all equal)
RoundTrip/ContainersOfContainers/Decode-4                  872 ± 0%       306 ± 0%  -64.91%  (p=0.000 n=10+10)
RoundTrip/ContainersOfContainers/Streaming_Decode-4        146 ± 0%       146 ± 0%     ~     (p=0.059 n=10+8)
```
  • Loading branch information
abhinav committed Jul 8, 2021
1 parent a37f2bc commit fae4d5f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 21 deletions.
22 changes: 6 additions & 16 deletions protocol/binary/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,8 @@ func (r *reader) readMapStream() (wire.MapItemList, error) {
}

start := r.or.offset
for i := 0; i < mh.Length; i++ {
if err := r.sr.Skip(mh.KeyType); err != nil {
return nil, err
}

if err := r.sr.Skip(mh.ValueType); err != nil {
return nil, err
}
if err := r.sr.skipMapItems(mh.KeyType, mh.ValueType, int32(mh.Length)); err != nil {
return nil, err
}

if err := r.sr.ReadMapEnd(); err != nil {
Expand All @@ -131,10 +125,8 @@ func (r *reader) readListStream() (wire.ValueList, error) {
}

start := r.or.offset
for i := 0; i < lh.Length; i++ {
if err := r.sr.Skip(lh.Type); err != nil {
return nil, err
}
if err := r.sr.skipListItems(lh.Type, lh.Length); err != nil {
return nil, err
}

if err := r.sr.ReadListEnd(); err != nil {
Expand All @@ -157,10 +149,8 @@ func (r *reader) readSetStream() (wire.ValueList, error) {
}

start := r.or.offset
for i := 0; i < sh.Length; i++ {
if err := r.sr.Skip(sh.Type); err != nil {
return nil, err
}
if err := r.sr.skipListItems(sh.Type, sh.Length); err != nil {
return nil, err
}

if err := r.sr.ReadSetEnd(); err != nil {
Expand Down
15 changes: 10 additions & 5 deletions protocol/binary/stream_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,12 +392,12 @@ func (sr *StreamReader) skipStruct() error {
}

func (sr *StreamReader) skipMap() error {
keyRaw, err := sr.ReadInt8()
key, err := sr.ReadInt8()
if err != nil {
return err
}

valueRaw, err := sr.ReadInt8()
value, err := sr.ReadInt8()
if err != nil {
return err
}
Expand All @@ -411,11 +411,12 @@ func (sr *StreamReader) skipMap() error {
return decodeErrorf("got negative length: %v", size)
}

key := wire.Type(keyRaw)
return sr.skipMapItems(wire.Type(key), wire.Type(value), size)
}

func (sr *StreamReader) skipMapItems(key, value wire.Type, size int32) error {
keyWidth := fixedWidth(key)
value := wire.Type(valueRaw)
valueWidth := fixedWidth(value)

if keyWidth > 0 && valueWidth > 0 {
length := int64(size) * (keyWidth + valueWidth)
return sr.discard(length)
Expand All @@ -440,6 +441,10 @@ func (sr *StreamReader) skipList() error {
return err
}

return sr.skipListItems(elemType, size)
}

func (sr *StreamReader) skipListItems(elemType wire.Type, size int) error {
width := fixedWidth(elemType)
if width > 0 {
length := width * int64(size)
Expand Down

0 comments on commit fae4d5f

Please sign in to comment.