Skip to content

Commit 3fb7836

Browse files
vdarulisvdarulis
authored
s2: check for cap, not len of buffer in EncodeBetter/Best (#1080)
* Consistently check for capacity of provided buffer * Make sure benchmarks reuse memory * Fix snapref, too
1 parent 33f59b4 commit 3fb7836

File tree

3 files changed

+49
-23
lines changed

3 files changed

+49
-23
lines changed

‎internal/snapref/encode.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ import (
2020
func Encode(dst, src []byte) []byte {
2121
if n := MaxEncodedLen(len(src)); n < 0 {
2222
panic(ErrTooLarge)
23-
} else if len(dst) < n {
23+
} else if cap(dst) < n {
2424
dst = make([]byte, n)
25+
} else {
26+
dst = dst[:n]
2527
}
2628

2729
// The block starts with the varint-encoded length of the decompressed bytes.

‎s2/encode.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,10 @@ func EstimateBlockSize(src []byte) (d int) {
117117
func EncodeBetter(dst, src []byte) []byte {
118118
if n := MaxEncodedLen(len(src)); n < 0 {
119119
panic(ErrTooLarge)
120-
} else if len(dst) < n {
120+
} else if cap(dst) < n {
121121
dst = make([]byte, n)
122+
} else {
123+
dst = dst[:n]
122124
}
123125

124126
// The block starts with the varint-encoded length of the decompressed bytes.
@@ -159,8 +161,10 @@ func EncodeBetter(dst, src []byte) []byte {
159161
func EncodeBest(dst, src []byte) []byte {
160162
if n := MaxEncodedLen(len(src)); n < 0 {
161163
panic(ErrTooLarge)
162-
} else if len(dst) < n {
164+
} else if cap(dst) < n {
163165
dst = make([]byte, n)
166+
} else {
167+
dst = dst[:n]
164168
}
165169

166170
// The block starts with the varint-encoded length of the decompressed bytes.

‎s2/s2_test.go‎

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,11 +1388,13 @@ func testSnappyDecode(t *testing.T, src []byte) {
13881388
func benchDecode(b *testing.B, src []byte) {
13891389
b.Run("default", func(b *testing.B) {
13901390
encoded := Encode(nil, src)
1391+
buf := make([]byte, 0, len(src))
13911392
b.SetBytes(int64(len(src)))
13921393
b.ReportAllocs()
13931394
b.ResetTimer()
1395+
var err error
13941396
for i := 0; i < b.N; i++ {
1395-
_, err := Decode(src[:0], encoded)
1397+
buf, err = Decode(buf[:0], encoded)
13961398
if err != nil {
13971399
b.Fatal(err)
13981400
}
@@ -1401,11 +1403,13 @@ func benchDecode(b *testing.B, src []byte) {
14011403
})
14021404
b.Run("better", func(b *testing.B) {
14031405
encoded := EncodeBetter(nil, src)
1406+
buf := make([]byte, 0, len(src))
14041407
b.SetBytes(int64(len(src)))
14051408
b.ReportAllocs()
14061409
b.ResetTimer()
1410+
var err error
14071411
for i := 0; i < b.N; i++ {
1408-
_, err := Decode(src[:0], encoded)
1412+
buf, err = Decode(buf[:0], encoded)
14091413
if err != nil {
14101414
b.Fatal(err)
14111415
}
@@ -1414,11 +1418,13 @@ func benchDecode(b *testing.B, src []byte) {
14141418
})
14151419
b.Run("best", func(b *testing.B) {
14161420
encoded := EncodeBest(nil, src)
1421+
buf := make([]byte, 0, len(src))
14171422
b.SetBytes(int64(len(src)))
14181423
b.ReportAllocs()
14191424
b.ResetTimer()
1425+
var err error
14201426
for i := 0; i < b.N; i++ {
1421-
_, err := Decode(src[:0], encoded)
1427+
buf, err = Decode(buf[:0], encoded)
14221428
if err != nil {
14231429
b.Fatal(err)
14241430
}
@@ -1427,11 +1433,13 @@ func benchDecode(b *testing.B, src []byte) {
14271433
})
14281434
b.Run("snappy-input", func(b *testing.B) {
14291435
encoded := snapref.Encode(nil, src)
1436+
buf := make([]byte, 0, len(src))
14301437
b.SetBytes(int64(len(src)))
14311438
b.ReportAllocs()
14321439
b.ResetTimer()
1440+
var err error
14331441
for i := 0; i < b.N; i++ {
1434-
_, err := Decode(src[:0], encoded)
1442+
buf, err = Decode(buf[:0], encoded)
14351443
if err != nil {
14361444
b.Fatal(err)
14371445
}
@@ -1442,63 +1450,75 @@ func benchDecode(b *testing.B, src []byte) {
14421450

14431451
func benchEncode(b *testing.B, src []byte) {
14441452
// Bandwidth is in amount of uncompressed data.
1445-
dst := make([]byte, snapref.MaxEncodedLen(len(src)))
1446-
b.ResetTimer()
14471453
b.Run("default", func(b *testing.B) {
1454+
dst := make([]byte, MaxEncodedLen(len(src)))
1455+
b.ResetTimer()
14481456
b.SetBytes(int64(len(src)))
14491457
b.ReportAllocs()
14501458
for i := 0; i < b.N; i++ {
1451-
Encode(dst, src)
1459+
dst = Encode(dst[:0], src)
14521460
}
1453-
b.ReportMetric(100*float64(len(Encode(dst, src)))/float64(len(src)), "pct")
1461+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
14541462
})
14551463
b.Run("better", func(b *testing.B) {
1464+
dst := make([]byte, MaxEncodedLen(len(src)))
1465+
b.ResetTimer()
14561466
b.SetBytes(int64(len(src)))
14571467
b.ReportAllocs()
14581468
for i := 0; i < b.N; i++ {
1459-
EncodeBetter(dst, src)
1469+
dst = EncodeBetter(dst[:0], src)
14601470
}
1461-
b.ReportMetric(100*float64(len(EncodeBetter(dst, src)))/float64(len(src)), "pct")
1471+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
14621472
})
14631473
b.Run("best", func(b *testing.B) {
1474+
dst := make([]byte, MaxEncodedLen(len(src)))
1475+
b.ResetTimer()
14641476
b.SetBytes(int64(len(src)))
14651477
b.ReportAllocs()
14661478
for i := 0; i < b.N; i++ {
1467-
EncodeBest(dst, src)
1479+
dst = EncodeBest(dst[:0], src)
14681480
}
1469-
b.ReportMetric(100*float64(len(EncodeBest(dst, src)))/float64(len(src)), "pct")
1481+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
14701482
})
14711483
b.Run("snappy-default", func(b *testing.B) {
1484+
dst := make([]byte, MaxEncodedLen(len(src)))
14721485
b.SetBytes(int64(len(src)))
14731486
b.ReportAllocs()
1487+
b.ResetTimer()
14741488
for i := 0; i < b.N; i++ {
1475-
EncodeSnappy(dst, src)
1489+
dst = EncodeSnappy(dst[:0], src)
14761490
}
1477-
b.ReportMetric(100*float64(len(EncodeSnappy(dst, src)))/float64(len(src)), "pct")
1491+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
14781492
})
14791493
b.Run("snappy-better", func(b *testing.B) {
1494+
dst := make([]byte, MaxEncodedLen(len(src)))
14801495
b.SetBytes(int64(len(src)))
14811496
b.ReportAllocs()
1497+
b.ResetTimer()
14821498
for i := 0; i < b.N; i++ {
1483-
EncodeSnappyBetter(dst, src)
1499+
dst = EncodeSnappyBetter(dst[:0], src)
14841500
}
1485-
b.ReportMetric(100*float64(len(EncodeSnappyBetter(dst, src)))/float64(len(src)), "pct")
1501+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
14861502
})
14871503
b.Run("snappy-best", func(b *testing.B) {
1504+
dst := make([]byte, MaxEncodedLen(len(src)))
14881505
b.SetBytes(int64(len(src)))
14891506
b.ReportAllocs()
1507+
b.ResetTimer()
14901508
for i := 0; i < b.N; i++ {
1491-
EncodeSnappyBest(dst, src)
1509+
dst = EncodeSnappyBest(dst[:0], src)
14921510
}
1493-
b.ReportMetric(100*float64(len(EncodeSnappyBest(dst, src)))/float64(len(src)), "pct")
1511+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
14941512
})
14951513
b.Run("snappy-ref-noasm", func(b *testing.B) {
1514+
dst := make([]byte, snapref.MaxEncodedLen(len(src)))
14961515
b.SetBytes(int64(len(src)))
14971516
b.ReportAllocs()
1517+
b.ResetTimer()
14981518
for i := 0; i < b.N; i++ {
1499-
snapref.Encode(dst, src)
1519+
dst = snapref.Encode(dst[:0], src)
15001520
}
1501-
b.ReportMetric(100*float64(len(snapref.Encode(dst, src)))/float64(len(src)), "pct")
1521+
b.ReportMetric(100*float64(len(dst))/float64(len(src)), "pct")
15021522
})
15031523
}
15041524

0 commit comments

Comments
 (0)