From 503ffba49af1275a27e4d33a2fcf7b1186efe2a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Rabenstein?= Date: Thu, 20 Oct 2022 11:38:01 +0200 Subject: [PATCH] chunkenc: Slightly optimize xorWrite/xoRead (#11476) With these changes, the "happy path" when the leading and trailing number of bits don't need an update, fewer operations are needed. The change is probably very marginal (no change in the benchmark added here, but the benchmark also doesn't cover non-changing values), and an argument could me made that avoiding pointers also has its benefits. However, I think that reducing the number of return values improves readability. Which convinced me that I should at least propose this. Signed-off-by: beorn7 --- tsdb/chunkenc/histogram.go | 5 ++- tsdb/chunkenc/xor.go | 69 +++++++++++++++++++------------------- tsdb/chunkenc/xor_test.go | 43 ++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 38 deletions(-) create mode 100644 tsdb/chunkenc/xor_test.go diff --git a/tsdb/chunkenc/histogram.go b/tsdb/chunkenc/histogram.go index aeabd256f7..6e326888af 100644 --- a/tsdb/chunkenc/histogram.go +++ b/tsdb/chunkenc/histogram.go @@ -528,7 +528,7 @@ func (a *HistogramAppender) Recode( } func (a *HistogramAppender) writeSumDelta(v float64) { - a.leading, a.trailing = xorWrite(a.b, v, a.sum, a.leading, a.trailing) + xorWrite(a.b, v, a.sum, &a.leading, &a.trailing) } type histogramIterator struct { @@ -867,11 +867,10 @@ func (it *histogramIterator) Next() ValueType { } func (it *histogramIterator) readSum() bool { - sum, leading, trailing, err := xorRead(&it.br, it.sum, it.leading, it.trailing) + err := xorRead(&it.br, &it.sum, &it.leading, &it.trailing) if err != nil { it.err = err return false } - it.sum, it.leading, it.trailing = sum, leading, trailing return true } diff --git a/tsdb/chunkenc/xor.go b/tsdb/chunkenc/xor.go index cf0a265d27..10d650d596 100644 --- a/tsdb/chunkenc/xor.go +++ b/tsdb/chunkenc/xor.go @@ -222,7 +222,7 @@ func bitRange(x int64, nbits uint8) bool { } func (a *xorAppender) writeVDelta(v float64) { - a.leading, a.trailing = xorWrite(a.b, v, a.v, a.leading, a.trailing) + xorWrite(a.b, v, a.v, &a.leading, &a.trailing) } type xorIterator struct { @@ -386,44 +386,42 @@ func (it *xorIterator) Next() ValueType { } func (it *xorIterator) readValue() ValueType { - val, leading, trailing, err := xorRead(&it.br, it.val, it.leading, it.trailing) + err := xorRead(&it.br, &it.val, &it.leading, &it.trailing) if err != nil { it.err = err return ValNone } - it.val, it.leading, it.trailing = val, leading, trailing it.numRead++ return ValFloat } -func xorWrite( - b *bstream, - newValue, currentValue float64, - currentLeading, currentTrailing uint8, -) (newLeading, newTrailing uint8) { +func xorWrite(b *bstream, newValue, currentValue float64, leading, trailing *uint8) { delta := math.Float64bits(newValue) ^ math.Float64bits(currentValue) if delta == 0 { b.writeBit(zero) - return currentLeading, currentTrailing + return } b.writeBit(one) - newLeading = uint8(bits.LeadingZeros64(delta)) - newTrailing = uint8(bits.TrailingZeros64(delta)) + newLeading := uint8(bits.LeadingZeros64(delta)) + newTrailing := uint8(bits.TrailingZeros64(delta)) // Clamp number of leading zeros to avoid overflow when encoding. if newLeading >= 32 { newLeading = 31 } - if currentLeading != 0xff && newLeading >= currentLeading && newTrailing >= currentTrailing { + if *leading != 0xff && newLeading >= *leading && newTrailing >= *trailing { // In this case, we stick with the current leading/trailing. b.writeBit(zero) - b.writeBits(delta>>currentTrailing, 64-int(currentLeading)-int(currentTrailing)) - return currentLeading, currentTrailing + b.writeBits(delta>>*trailing, 64-int(*leading)-int(*trailing)) + return } + // Update leading/trailing for the caller. + *leading, *trailing = newLeading, newTrailing + b.writeBit(one) b.writeBits(uint64(newLeading), 5) @@ -435,42 +433,43 @@ func xorWrite( sigbits := 64 - newLeading - newTrailing b.writeBits(uint64(sigbits), 6) b.writeBits(delta>>newTrailing, int(sigbits)) - return } -func xorRead( - br *bstreamReader, currentValue float64, currentLeading, currentTrailing uint8, -) (newValue float64, newLeading, newTrailing uint8, err error) { - var bit bit - var bits uint64 - - bit, err = br.readBitFast() +func xorRead(br *bstreamReader, value *float64, leading, trailing *uint8) error { + bit, err := br.readBitFast() if err != nil { bit, err = br.readBit() } if err != nil { - return + return err } if bit == zero { - return currentValue, currentLeading, currentTrailing, nil + return nil } bit, err = br.readBitFast() if err != nil { bit, err = br.readBit() } if err != nil { - return + return err } + + var ( + bits uint64 + newLeading, newTrailing, mbits uint8 + ) + if bit == zero { // Reuse leading/trailing zero bits. - newLeading, newTrailing = currentLeading, currentTrailing + newLeading, newTrailing = *leading, *trailing + mbits = 64 - newLeading - newTrailing } else { bits, err = br.readBitsFast(5) if err != nil { bits, err = br.readBits(5) } if err != nil { - return + return err } newLeading = uint8(bits) @@ -479,29 +478,29 @@ func xorRead( bits, err = br.readBits(6) } if err != nil { - return + return err } - mbits := uint8(bits) + mbits = uint8(bits) // 0 significant bits here means we overflowed and we actually // need 64; see comment in xrWrite. if mbits == 0 { mbits = 64 } newTrailing = 64 - newLeading - mbits + // Update leading/trailing zero bits for the caller. + *leading, *trailing = newLeading, newTrailing } - - mbits := 64 - newLeading - newTrailing bits, err = br.readBitsFast(mbits) if err != nil { bits, err = br.readBits(mbits) } if err != nil { - return + return err } - vbits := math.Float64bits(currentValue) + vbits := math.Float64bits(*value) vbits ^= bits << newTrailing - newValue = math.Float64frombits(vbits) - return + *value = math.Float64frombits(vbits) + return nil } // OOOXORChunk holds a XORChunk and overrides the Encoding() method. diff --git a/tsdb/chunkenc/xor_test.go b/tsdb/chunkenc/xor_test.go new file mode 100644 index 0000000000..c7010ccd98 --- /dev/null +++ b/tsdb/chunkenc/xor_test.go @@ -0,0 +1,43 @@ +// Copyright 2022 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package chunkenc + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func BenchmarkXorRead(b *testing.B) { + c := NewXORChunk() + app, err := c.Appender() + require.NoError(b, err) + for i := int64(0); i < 120*1000; i += 1000 { + app.Append(i, float64(i)+float64(i)/10+float64(i)/100+float64(i)/1000) + } + + b.ReportAllocs() + b.ResetTimer() + + var it Iterator + for i := 0; i < b.N; i++ { + var ts int64 + var v float64 + it = c.Iterator(it) + for it.Next() != ValNone { + ts, v = it.At() + } + _, _ = ts, v + } +}