From ac6ace8dbdd77ee799ec637265fe24e2f79462fd Mon Sep 17 00:00:00 2001 From: Cyril Tovena Date: Thu, 29 Jul 2021 09:18:21 +0200 Subject: [PATCH] Make `ParseExternalKey` from the chunk package alloc free. (#4075) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` ~/go/src/github.com/grafana/loki main* ⇣ ❯ benchcmp before.txt after.txt2 benchmark old ns/op new ns/op delta Benchmark_ParseExternalKey-16 569 415 -27.03% benchmark old allocs new allocs delta Benchmark_ParseExternalKey-16 2 0 -100.00% benchmark old bytes new bytes delta Benchmark_ParseExternalKey-16 96 0 -100.00% ``` This is a very hot function in Loki being used for every chunk reference we get from the index. Signed-off-by: Cyril Tovena --- pkg/storage/batch_test.go | 1 + pkg/storage/chunk/chunk.go | 65 ++++++++++++++++++++++++++------- pkg/storage/chunk/chunk_test.go | 7 ++++ 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/pkg/storage/batch_test.go b/pkg/storage/batch_test.go index dd67ac4f11..fc69d3f1ad 100644 --- a/pkg/storage/batch_test.go +++ b/pkg/storage/batch_test.go @@ -1522,6 +1522,7 @@ var entry logproto.Entry func Benchmark_store_OverlappingChunks(b *testing.B) { b.ReportAllocs() st := &store{ + chunkMetrics: NilMetrics, cfg: Config{ MaxChunkBatchSize: 50, }, diff --git a/pkg/storage/chunk/chunk.go b/pkg/storage/chunk/chunk.go index 5da290fb3b..74427f1b4f 100644 --- a/pkg/storage/chunk/chunk.go +++ b/pkg/storage/chunk/chunk.go @@ -5,9 +5,11 @@ import ( "encoding/binary" "fmt" "hash/crc32" + "reflect" "strconv" "strings" "sync" + "unsafe" "github.com/cortexproject/cortex/pkg/prom1/storage/metric" "github.com/golang/snappy" @@ -91,13 +93,11 @@ func ParseExternalKey(userID, externalKey string) (Chunk, error) { if !strings.Contains(externalKey, "/") { return parseLegacyChunkID(userID, externalKey) } - chunk, err := parseNewExternalKey(externalKey) + chunk, err := parseNewExternalKey(userID, externalKey) if err != nil { return Chunk{}, err } - if chunk.UserID != userID { - return Chunk{}, errors.WithStack(ErrWrongMetadata) - } + return chunk, nil } @@ -126,29 +126,43 @@ func parseLegacyChunkID(userID, key string) (Chunk, error) { }, nil } -func parseNewExternalKey(key string) (Chunk, error) { - parts := strings.Split(key, "/") - if len(parts) != 2 { +func parseNewExternalKey(userID, key string) (Chunk, error) { + userIdx := strings.Index(key, "/") + if userIdx == -1 || userIdx+1 >= len(key) { return Chunk{}, errInvalidChunkID(key) } - userID := parts[0] - hexParts := strings.Split(parts[1], ":") - if len(hexParts) != 4 { + if userID != key[:userIdx] { + return Chunk{}, errors.WithStack(ErrWrongMetadata) + } + hexParts := key[userIdx+1:] + partsBytes := unsafeGetBytes(hexParts) + h0, i := readOneHexPart(partsBytes) + if i == 0 || i+1 >= len(partsBytes) { return Chunk{}, errInvalidChunkID(key) } - fingerprint, err := strconv.ParseUint(hexParts[0], 16, 64) + fingerprint, err := strconv.ParseUint(unsafeGetString(h0), 16, 64) if err != nil { return Chunk{}, err } - from, err := strconv.ParseInt(hexParts[1], 16, 64) + partsBytes = partsBytes[i+1:] + h1, i := readOneHexPart(partsBytes) + if i == 0 || i+1 >= len(partsBytes) { + return Chunk{}, errInvalidChunkID(key) + } + from, err := strconv.ParseInt(unsafeGetString(h1), 16, 64) if err != nil { return Chunk{}, err } - through, err := strconv.ParseInt(hexParts[2], 16, 64) + partsBytes = partsBytes[i+1:] + h2, i := readOneHexPart(partsBytes) + if i == 0 || i+1 >= len(partsBytes) { + return Chunk{}, errInvalidChunkID(key) + } + through, err := strconv.ParseInt(unsafeGetString(h2), 16, 64) if err != nil { return Chunk{}, err } - checksum, err := strconv.ParseUint(hexParts[3], 16, 32) + checksum, err := strconv.ParseUint(unsafeGetString(partsBytes[i+1:]), 16, 32) if err != nil { return Chunk{}, err } @@ -162,6 +176,29 @@ func parseNewExternalKey(key string) (Chunk, error) { }, nil } +func readOneHexPart(hex []byte) (part []byte, i int) { + for i < len(hex) { + if hex[i] != ':' { + i++ + continue + } + return hex[:i], i + } + return nil, 0 +} + +func unsafeGetBytes(s string) []byte { + var buf []byte + p := unsafe.Pointer(&buf) + *(*string)(p) = s + (*reflect.SliceHeader)(p).Cap = len(s) + return buf +} + +func unsafeGetString(buf []byte) string { + return *((*string)(unsafe.Pointer(&buf))) +} + // ExternalKey returns the key you can use to fetch this chunk from external // storage. For newer chunks, this key includes a checksum. func (c *Chunk) ExternalKey() string { diff --git a/pkg/storage/chunk/chunk_test.go b/pkg/storage/chunk/chunk_test.go index 59d9862c66..da4949a248 100644 --- a/pkg/storage/chunk/chunk_test.go +++ b/pkg/storage/chunk/chunk_test.go @@ -378,3 +378,10 @@ func TestChunk_Slice(t *testing.T) { }) } } + +func Benchmark_ParseExternalKey(b *testing.B) { + for i := 0; i < b.N; i++ { + _, err := ParseExternalKey("fake", "fake/57f628c7f6d57aad:162c699f000:162c69a07eb:eb242d99") + require.NoError(b, err) + } +}