chunkenc: make xor reading more DRY

Signed-off-by: beorn7 <beorn@grafana.com>

tsdb/chunkenc/histogram.go

@@ -838,69 +838,11 @@ func (it *histogramIterator) Next() bool {
 }
 
 func (it *histogramIterator) readSum() bool {
-	bit, err := it.br.readBitFast()
+	sum, leading, trailing, err := xorRead(&it.br, it.sum, it.leading, it.trailing)
-	if err != nil {
-		bit, err = it.br.readBit()
-	}
 	if err != nil {
 		it.err = err
 		return false
 	}
-
+	it.sum, it.leading, it.trailing = sum, leading, trailing
-	if bit == zero {
-		return true // it.sum = it.sum
-	}
-
-	bit, err = it.br.readBitFast()
-	if err != nil {
-		bit, err = it.br.readBit()
-	}
-	if err != nil {
-		it.err = err
-		return false
-	}
-	if bit == zero {
-		// Reuse leading/trailing zero bits.
-		// it.leading, it.trailing = it.leading, it.trailing
-	} else {
-		bits, err := it.br.readBitsFast(5)
-		if err != nil {
-			bits, err = it.br.readBits(5)
-		}
-		if err != nil {
-			it.err = err
-			return false
-		}
-		it.leading = uint8(bits)
-
-		bits, err = it.br.readBitsFast(6)
-		if err != nil {
-			bits, err = it.br.readBits(6)
-		}
-		if err != nil {
-			it.err = err
-			return false
-		}
-		mbits := uint8(bits)
-		// 0 significant bits here means we overflowed and we actually
-		// need 64; see comment in encoder.
-		if mbits == 0 {
-			mbits = 64
-		}
-		it.trailing = 64 - it.leading - mbits
-	}
-
-	mbits := 64 - it.leading - it.trailing
-	bits, err := it.br.readBitsFast(mbits)
-	if err != nil {
-		bits, err = it.br.readBits(mbits)
-	}
-	if err != nil {
-		it.err = err
-		return false
-	}
-	vbits := math.Float64bits(it.sum)
-	vbits ^= bits << it.trailing
-	it.sum = math.Float64frombits(vbits)
 	return true
 }

tsdb/chunkenc/xor.go

@@ -221,45 +221,6 @@ func (a *xorAppender) writeVDelta(v float64) {
 	a.leading, a.trailing = xorWrite(a.b, v, a.v, a.leading, a.trailing)
 }
 
-func xorWrite(
-	b *bstream,
-	current, previous float64,
-	currentLeading, currentTrailing uint8,
-) (newLeading, newTrailing uint8) {
-	delta := math.Float64bits(current) ^ math.Float64bits(previous)
-
-	if delta == 0 {
-		b.writeBit(zero)
-		return
-	}
-	b.writeBit(one)
-
-	leading := uint8(bits.LeadingZeros64(delta))
-	trailing := uint8(bits.TrailingZeros64(delta))
-
-	// Clamp number of leading zeros to avoid overflow when encoding.
-	if leading >= 32 {
-		leading = 31
-	}
-
-	if currentLeading != 0xff && leading >= currentLeading && trailing >= currentTrailing {
-		b.writeBit(zero)
-		b.writeBits(delta>>currentTrailing, 64-int(currentLeading)-int(currentTrailing))
-		return currentLeading, currentTrailing
-	}
-
-	b.writeBit(one)
-	b.writeBits(uint64(leading), 5)
-
-	// Note that if leading == trailing == 0, then sigbits == 64.  But that value doesn't actually fit into the 6 bits we have.
-	// Luckily, we never need to encode 0 significant bits, since that would put us in the other case (vdelta == 0).
-	// So instead we write out a 0 and adjust it back to 64 on unpacking.
-	sigbits := 64 - leading - trailing
-	b.writeBits(uint64(sigbits), 6)
-	b.writeBits(delta>>trailing, int(sigbits))
-	return leading, trailing
-}
-
 type xorIterator struct {
 	br       bstreamReader
 	numTotal uint16
@@ -415,70 +376,117 @@ func (it *xorIterator) Next() bool {
 }
 
 func (it *xorIterator) readValue() bool {
-	bit, err := it.br.readBitFast()
+	val, leading, trailing, err := xorRead(&it.br, it.val, it.leading, it.trailing)
-	if err != nil {
-		bit, err = it.br.readBit()
-	}
 	if err != nil {
 		it.err = err
 		return false
 	}
-
+	it.val, it.leading, it.trailing = val, leading, trailing
-	if bit == zero {
-		// it.val = it.val
-	} else {
-		bit, err := it.br.readBitFast()
-		if err != nil {
-			bit, err = it.br.readBit()
-		}
-		if err != nil {
-			it.err = err
-			return false
-		}
-		if bit == zero {
-			// reuse leading/trailing zero bits
-			// it.leading, it.trailing = it.leading, it.trailing
-		} else {
-			bits, err := it.br.readBitsFast(5)
-			if err != nil {
-				bits, err = it.br.readBits(5)
-			}
-			if err != nil {
-				it.err = err
-				return false
-			}
-			it.leading = uint8(bits)
-
-			bits, err = it.br.readBitsFast(6)
-			if err != nil {
-				bits, err = it.br.readBits(6)
-			}
-			if err != nil {
-				it.err = err
-				return false
-			}
-			mbits := uint8(bits)
-			// 0 significant bits here means we overflowed and we actually need 64; see comment in encoder
-			if mbits == 0 {
-				mbits = 64
-			}
-			it.trailing = 64 - it.leading - mbits
-		}
-
-		mbits := 64 - it.leading - it.trailing
-		bits, err := it.br.readBitsFast(mbits)
-		if err != nil {
-			bits, err = it.br.readBits(mbits)
-		}
-		if err != nil {
-			it.err = err
-			return false
-		}
-		vbits := math.Float64bits(it.val)
-		vbits ^= bits << it.trailing
-		it.val = math.Float64frombits(vbits)
-	}
-
 	it.numRead++
 	return true
 }
+
+func xorWrite(
+	b *bstream,
+	current, previous float64,
+	currentLeading, currentTrailing uint8,
+) (newLeading, newTrailing uint8) {
+	delta := math.Float64bits(current) ^ math.Float64bits(previous)
+
+	if delta == 0 {
+		b.writeBit(zero)
+		return
+	}
+	b.writeBit(one)
+
+	leading := uint8(bits.LeadingZeros64(delta))
+	trailing := uint8(bits.TrailingZeros64(delta))
+
+	// Clamp number of leading zeros to avoid overflow when encoding.
+	if leading >= 32 {
+		leading = 31
+	}
+
+	if currentLeading != 0xff && leading >= currentLeading && trailing >= currentTrailing {
+		b.writeBit(zero)
+		b.writeBits(delta>>currentTrailing, 64-int(currentLeading)-int(currentTrailing))
+		return currentLeading, currentTrailing
+	}
+
+	b.writeBit(one)
+	b.writeBits(uint64(leading), 5)
+
+	// Note that if leading == trailing == 0, then sigbits == 64.  But that value doesn't actually fit into the 6 bits we have.
+	// Luckily, we never need to encode 0 significant bits, since that would put us in the other case (vdelta == 0).
+	// So instead we write out a 0 and adjust it back to 64 on unpacking.
+	sigbits := 64 - leading - trailing
+	b.writeBits(uint64(sigbits), 6)
+	b.writeBits(delta>>trailing, int(sigbits))
+	return leading, trailing
+}
+
+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()
+	if err != nil {
+		bit, err = br.readBit()
+	}
+	if err != nil {
+		return
+	}
+	if bit == zero {
+		return currentValue, currentLeading, currentTrailing, nil
+	}
+	bit, err = br.readBitFast()
+	if err != nil {
+		bit, err = br.readBit()
+	}
+	if err != nil {
+		return
+	}
+	if bit == zero {
+		// Reuse leading/trailing zero bits.
+		newLeading, newTrailing = currentLeading, currentTrailing
+	} else {
+		bits, err = br.readBitsFast(5)
+		if err != nil {
+			bits, err = br.readBits(5)
+		}
+		if err != nil {
+			return
+		}
+		newLeading = uint8(bits)
+
+		bits, err = br.readBitsFast(6)
+		if err != nil {
+			bits, err = br.readBits(6)
+		}
+		if err != nil {
+			return
+		}
+		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
+	}
+
+	mbits := 64 - newLeading - newTrailing
+	bits, err = br.readBitsFast(mbits)
+	if err != nil {
+		bits, err = br.readBits(mbits)
+	}
+	if err != nil {
+		return
+	}
+	vbits := math.Float64bits(currentValue)
+	vbits ^= bits << newTrailing
+	newValue = math.Float64frombits(vbits)
+	return
+}