From d5339521b84f48c4264ae4bb93304b5c19e6c016 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Wed, 18 Dec 2024 07:44:49 -0800 Subject: [PATCH] valblk: fix Writer.Size() performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Writer.Size()` unnecessarily loops through all the blocks instead of using `totalBlockBytes`. This seems like an oversight: we only append to `blocks` in `compressAndFlush` where we also update `totalBlockBytes`. I verified that in all Pebble tests these values are the same, using `if sz != w.totalBlockBytes { panic() }`. ``` name old time/op new time/op delta WriterWithVersions/vals-per-key=1/format=(Pebble,v5)/block=32KB/filter=false/compression=NoCompression 77.2ms ± 1% 76.6ms ± 0% -0.70% (p=0.008 n=5+5) WriterWithVersions/vals-per-key=10/format=(Pebble,v5)/block=32KB/filter=false/compression=NoCompression 752ms ± 0% 99ms ± 1% -86.83% (p=0.008 n=5+5) WriterWithVersions/vals-per-key=10000/format=(Pebble,v5)/block=32KB/filter=false/compression=NoCompression 908ms ± 0% 103ms ± 0% -88.64% (p=0.008 n=5+5) name old speed new speed delta WriterWithVersions/vals-per-key=1/format=(Pebble,v5)/block=32KB/filter=false/compression=NoCompression 462MB/s ± 1% 466MB/s ± 0% +0.70% (p=0.008 n=5+5) WriterWithVersions/vals-per-key=10/format=(Pebble,v5)/block=32KB/filter=false/compression=NoCompression 53.0MB/s ± 0% 402.7MB/s ± 1% +659.47% (p=0.008 n=5+5) WriterWithVersions/vals-per-key=10000/format=(Pebble,v5)/block=32KB/filter=false/compression=NoCompression 49.1MB/s ± 0% 431.8MB/s ± 0% +780.16% (p=0.008 n=5+5) ``` --- sstable/valblk/writer.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sstable/valblk/writer.go b/sstable/valblk/writer.go index ab017dba4f..892ac9ef50 100644 --- a/sstable/valblk/writer.go +++ b/sstable/valblk/writer.go @@ -161,10 +161,7 @@ func (w *Writer) AddValue(v []byte) (Handle, error) { // Size returns the total size of currently buffered value blocks. func (w *Writer) Size() uint64 { - var sz uint64 - for _, blk := range w.blocks { - sz += uint64(blk.block.LengthWithTrailer()) - } + sz := w.totalBlockBytes if w.buf != nil { sz += uint64(len(w.buf.b)) }