From 8d2ae405e8f2ba4b9bdb09866f563053375a908b Mon Sep 17 00:00:00 2001 From: Alex B <142788550+A13xB0@users.noreply.github.com> Date: Tue, 13 Oct 2026 08:01:57 +0100 Subject: [PATCH] perf(store): cache an observation's parsed time in 8 bytes (#2196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #2148. ## Problem `StoreObs` is the store's most numerous struct: 400k of them on a week of a busy regional network. Its lazy timestamp cache (#1481) is a `time.Time`, a `sync.Once` and a `bool`, together 40 bytes. On current master that makes the struct 200 bytes, which puts every observation in the 208-byte size class. ## Change The cache is now a single `atomic.Int64`: - `0` before the first call; - the parsed time as UTC nanoseconds after it; - or one of two sentinels: one for empty or unparseable strings, and one for times it can't hold exactly (a non-UTC offset, or outside `UnixNano`'s range). Those are re-parsed on each call so their location is preserved. Concurrent first calls may both parse; they store the same value. `StoreObs` is now 160 bytes, in the 160-byte size class: 48 bytes × 400k observations ≈ **19 MB** less heap. The first parse costs the same; cached reads now build the `time.Time` from an int. ## Tests - `TestStoreObsFitsIn160Bytes`: red commit first. - `TestStoreObsParsedTimeMatchesParse`: Z, nanosecond, offset, SQL, empty, garbage and epoch inputs give the same time and location as `time.Parse`, on the first and the cached call. - `TestStoreObsParsedTimeConcurrentFill`. - The existing `storeobs_parsedtime_test.go` still passes. - `go test ./cmd/server` passes, and `-race` passes on the ParsedTime, retransmission and observer analytics tests. ## This PR or #2149 #2149 fixes #2148 by reordering these same three fields. The two conflict, so here is how they compare. Sizes are `unsafe.Sizeof(StoreObs{})` on each branch, with Go's heap size class beside them: | | struct | heap per observation | saved per observation | saved at 400k observations | |---|---|---|---|---| | master | 200 B | 208 B | – | – | | #2149 (field reorder) | 192 B | 192 B | 16 B | ~6.4 MB | | this PR (one 8-byte field) | 160 B | 160 B | 48 B | ~19 MB | - **#2149:** a pure field reorder with no change in behaviour. It is the smallest, safest change and the easiest to review. It saves a third of what this PR does. - **This PR:** three times the saving, but more code. It adds two sentinel values, and re-parses on each call any timestamp with a non-UTC offset. The store writes UTC (`Z`), so that path is rare. It is tested against `time.Parse` on edge cases, concurrently, and under `-race`. Options: 1. **Take this PR.** #2149 then has nothing left to reorder. 2. **Take #2149 now, and this PR after it** as a follow-up for the further 32 bytes per observation. I'll rebase onto #2149 if it lands first. 3. **Take only #2149** if the smaller, simpler change is preferred. No config or UI changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 --- cmd/server/obs_parsed_time_test.go | 72 ++++++++++++++++++++++ cmd/server/retransmission_pressure_test.go | 3 +- cmd/server/store.go | 71 ++++++++++++++------- 3 files changed, 121 insertions(+), 25 deletions(-) create mode 100644 cmd/server/obs_parsed_time_test.go diff --git a/cmd/server/obs_parsed_time_test.go b/cmd/server/obs_parsed_time_test.go new file mode 100644 index 00000000..65aabb82 --- /dev/null +++ b/cmd/server/obs_parsed_time_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "sync" + "testing" + "time" + "unsafe" +) + +// StoreObs is the store's most numerous struct (400k on a week of a busy +// network). Its lazy timestamp cache used to be a time.Time, a sync.Once and +// a bool, 40 bytes that put each observation in the 192-byte size class. It +// is 8 bytes now, and each observation fits in 160. +func TestStoreObsFitsIn160Bytes(t *testing.T) { + if n := unsafe.Sizeof(StoreObs{}); n > 160 { + t.Fatalf("StoreObs is %d bytes, want at most 160 (one size class down from 192)", n) + } +} + +func parseObsTimestampReference(s string) (time.Time, bool) { + for _, layout := range []string{time.RFC3339Nano, time.RFC3339, "2006-01-02 15:04:05"} { + if t, err := time.Parse(layout, s); err == nil { + return t, true + } + } + return time.Time{}, false +} + +// ParsedTime gives the same time, location included, as parsing the string. +func TestStoreObsParsedTimeMatchesParse(t *testing.T) { + for _, ts := range []string{ + "2026-10-10T12:34:56.789Z", + "2026-10-10T12:34:56Z", + "2026-10-10T13:34:56+01:00", + "2026-10-10 12:34:56", + "", + "not a time", + "1970-01-01T00:00:00Z", + } { + o := &StoreObs{Timestamp: ts} + want, wantOK := parseObsTimestampReference(ts) + for i := 0; i < 2; i++ { // first call parses, second reads the cache + got, ok := o.ParsedTime() + if ok != wantOK || !got.Equal(want) || got.Location().String() != want.Location().String() { + t.Fatalf("%q call %d: got %v (%v) ok=%v, want %v (%v) ok=%v", ts, i, got, got.Location(), ok, want, want.Location(), wantOK) + } + if wantOK && got.Hour() != want.Hour() { + t.Fatalf("%q: hour %d, want %d", ts, got.Hour(), want.Hour()) + } + } + } +} + +// Concurrent readers under the store's read lock may race to fill the cache. +func TestStoreObsParsedTimeConcurrentFill(t *testing.T) { + o := &StoreObs{Timestamp: "2026-10-10T12:34:56.789Z"} + want, _ := parseObsTimestampReference(o.Timestamp) + var wg sync.WaitGroup + for i := 0; i < 16; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 100; j++ { + if got, ok := o.ParsedTime(); !ok || !got.Equal(want) { + t.Errorf("got %v ok=%v", got, ok) + return + } + } + }() + } + wg.Wait() +} diff --git a/cmd/server/retransmission_pressure_test.go b/cmd/server/retransmission_pressure_test.go index 9226ba27..0456f141 100644 --- a/cmd/server/retransmission_pressure_test.go +++ b/cmd/server/retransmission_pressure_test.go @@ -675,8 +675,7 @@ func BenchmarkComputeRetransmissionPressureColdTimestamps(b *testing.B) { b.StopTimer() for _, tx := range s.packets { for _, o := range tx.Observations { - o.tsParseOnce = sync.Once{} - o.tsParsed, o.tsParsedOK = time.Time{}, false + o.tsUnixNano.Store(0) } } b.StartTimer() diff --git a/cmd/server/store.go b/cmd/server/store.go index 04bb7d6d..8374e787 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -93,36 +93,61 @@ type StoreObs struct { // #1481 P0-2: cached parsed timestamp. The legacy handlers parse // Timestamp as RFC3339Nano/RFC3339/"2006-01-02 15:04:05" with a // fallback chain on every read; for /api/observers/{id}/analytics - // that fires 60k+ times per request under RLock. ParsedTime returns - // the parsed value once, caching for the lifetime of the StoreObs. - tsParseOnce sync.Once - tsParsed time.Time - tsParsedOK bool + // that fires 60k+ times per request under RLock. ParsedTime parses + // once and keeps the result here in 8 bytes (see tsUnparsed): a + // time.Time, sync.Once and bool took 40 and put each observation in + // the 192-byte size class instead of 160. + tsUnixNano atomic.Int64 } // ParsedTime returns the parsed Timestamp, caching the result. // Tries RFC3339Nano → RFC3339 → "2006-01-02 15:04:05" in order. // Returns (zero, false) if Timestamp is empty or unparseable. -// Thread-safe via sync.Once. +// Safe for concurrent use: callers racing on the first call each parse and +// store the same value. func (o *StoreObs) ParsedTime() (time.Time, bool) { - o.tsParseOnce.Do(func() { - if o.Timestamp == "" { - return + switch n := o.tsUnixNano.Load(); n { + case 0: + case tsUnparsed: + return time.Time{}, false + case tsReparse: + return parseObsTimestamp(o.Timestamp) + default: + return time.Unix(0, n).UTC(), true + } + t, ok := parseObsTimestamp(o.Timestamp) + switch { + case !ok: + o.tsUnixNano.Store(tsUnparsed) + case t.Location() != time.UTC || t.Year() < 1678 || t.Year() > 2261 || t.UnixNano() == 0: + // Kept only as UTC nanoseconds; a time with another offset (or one + // UnixNano cannot hold) is parsed again on each call so its + // location is kept. + o.tsUnixNano.Store(tsReparse) + default: + o.tsUnixNano.Store(t.UnixNano()) + return time.Unix(0, t.UnixNano()).UTC(), true + } + return t, ok +} + +// tsUnixNano holds 0 before the first ParsedTime, the UTC time in +// nanoseconds after it, or one of these. +const ( + tsUnparsed = math.MinInt64 // empty or not a time + tsReparse = math.MinInt64 + 1 // parsed, but not held as UTC nanoseconds +) + +func parseObsTimestamp(s string) (time.Time, bool) { + if s == "" { + return time.Time{}, false + } + for _, layout := range []string{time.RFC3339Nano, time.RFC3339, "2006-01-02 15:04:05"} { + if t, err := time.Parse(layout, s); err == nil { + return t, true } - if t, err := time.Parse(time.RFC3339Nano, o.Timestamp); err == nil { - o.tsParsed, o.tsParsedOK = t, true - return - } - if t, err := time.Parse(time.RFC3339, o.Timestamp); err == nil { - o.tsParsed, o.tsParsedOK = t, true - return - } - if t, err := time.Parse("2006-01-02 15:04:05", o.Timestamp); err == nil { - o.tsParsed, o.tsParsedOK = t, true - return - } - }) - return o.tsParsed, o.tsParsedOK + } + return time.Time{}, false } // ParsedDecoded returns the parsed DecodedJSON map, caching the result.