From cc56b680f4400576debd212d5905768f32e36cab Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:01:51 +0530 Subject: [PATCH 1/2] perf(tokenizer): read the clock only when metrics are enabled [W1-02] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 1 + pkg/sql/tokenizer/metrics_gate_test.go | 87 ++++++++++++++++++++++++++ pkg/sql/tokenizer/tokenizer.go | 24 ++++--- 3 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 pkg/sql/tokenizer/metrics_gate_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b30750cb..f453d960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Performance +- **Tokenizer**: `time.Now`/`time.Since` are now read only when metrics are enabled (previously twice per tokenization even when disabled). If metrics are toggled on mid-call the call is not recorded. No API or token-output change [W1-02]. - **Tokenizer**: keyword and compound-keyword-start lookup now uses a single fused-hash open-addressed table (one FNV-1a hash computed while upper-casing, one probe) instead of two Go map lookups per identifier. The table is filled in a fixed order (hot keywords first, then sorted) so its probe layout is deterministic across processes. No behaviour change; token output is identical [W2-01]. - **Pools**: the tokenizer, parser and AST pools now count Get/Put through precreated, cache-line padded atomic counters (`internal/poolstats`) instead of an RWMutex-guarded map keyed by pool name, removing a lock, map lookup and string hash from every pool operation. `metrics.GetPoolStats` and `metrics.ResetPoolStats` behave exactly as before; named custom pools keep the map path. [W1-01] - **Parser**: token preprocessing now takes a type-gated fast path. Only identifier, keyword, aggregate and compound tokens take the normalisation path; every other token is appended with a single copy. Output is identical (guarded by an old-versus-new differential test); no API or behaviour change. diff --git a/pkg/sql/tokenizer/metrics_gate_test.go b/pkg/sql/tokenizer/metrics_gate_test.go new file mode 100644 index 00000000..81abf1ae --- /dev/null +++ b/pkg/sql/tokenizer/metrics_gate_test.go @@ -0,0 +1,87 @@ +// Copyright 2026 GoSQLX Contributors +// SPDX-License-Identifier: Apache-2.0 + +package tokenizer + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/metrics" +) + +// TestTokenize_MetricsGate verifies tokenization is recorded when metrics are +// enabled and leaves every counter untouched when they are disabled. +func TestTokenize_MetricsGate(t *testing.T) { + was := metrics.IsEnabled() + t.Cleanup(func() { + if was { + metrics.Enable() + } else { + metrics.Disable() + } + }) + + run := func() { + tkz := GetTokenizer() + defer PutTokenizer(tkz) + if _, err := tkz.Tokenize([]byte("SELECT a, b FROM t WHERE c = 1")); err != nil { + t.Fatalf("tokenize: %v", err) + } + if _, err := tkz.Tokenize([]byte("SELECT 'unterminated")); err == nil { + t.Fatal("expected error for unterminated string") + } + if _, err := tkz.Tokenize([]byte(strings.Repeat("x", MaxInputSize+1))); err == nil { + t.Fatal("expected error for oversized input") + } + } + + metrics.Enable() + metrics.Reset() + run() + on := metrics.GetStats() + if on.TokenizeOperations != 3 { + t.Fatalf("enabled: TokenizeOperations = %d, want 3", on.TokenizeOperations) + } + if on.TokenizeErrors != 2 { + t.Fatalf("enabled: TokenizeErrors = %d, want 2", on.TokenizeErrors) + } + + metrics.Disable() + metrics.Reset() + run() + off := metrics.GetStats() + if off.TokenizeOperations != 0 || off.TokenizeErrors != 0 { + t.Fatalf("disabled: recorded ops=%d errs=%d, want 0", off.TokenizeOperations, off.TokenizeErrors) + } +} + +// TestTokenize_MetricsDoNotChangeTokens checks output is identical with +// metrics on and off. +func TestTokenize_MetricsDoNotChangeTokens(t *testing.T) { + was := metrics.IsEnabled() + t.Cleanup(func() { + if was { + metrics.Enable() + } else { + metrics.Disable() + } + }) + sql := []byte("SELECT a, COUNT(*) FROM t JOIN u ON t.id = u.id GROUP BY a") + get := func() int { + tkz := GetTokenizer() + defer PutTokenizer(tkz) + toks, err := tkz.Tokenize(sql) + if err != nil { + t.Fatal(err) + } + return len(toks) + } + metrics.Enable() + a := get() + metrics.Disable() + b := get() + if a != b { + t.Fatalf("token count differs: on=%d off=%d", a, b) + } +} diff --git a/pkg/sql/tokenizer/tokenizer.go b/pkg/sql/tokenizer/tokenizer.go index 745b90a2..cb439fbf 100644 --- a/pkg/sql/tokenizer/tokenizer.go +++ b/pkg/sql/tokenizer/tokenizer.go @@ -605,14 +605,22 @@ func (t *Tokenizer) load(input []byte, src string) { // the caller. When scratch is true (the internal tokscratch path) it is built // in t.tokBuf and stays owned by t; see tokBuf. func (t *Tokenizer) tokenize(ctx context.Context, input []byte, src string, haveSrc, scratch bool) ([]models.TokenWithSpan, error) { - // Record start time for metrics - startTime := time.Now() + // Read the clock only when metrics are enabled; time.Now is a measurable + // share of short tokenizations. If metrics are toggled on mid-call the + // duration is recorded as 0. + var startTime time.Time + metricsOn := metrics.IsEnabled() + if metricsOn { + startTime = time.Now() + } // Validate input size to prevent DoS attacks. This must stay ahead of the // string copy below so an oversized input is rejected before it is copied. if len(input) > MaxInputSize { err := errors.InputTooLargeError(int64(len(input)), MaxInputSize, models.Location{Line: 1, Column: 1}) - metrics.RecordTokenization(time.Since(startTime), len(input), err) + if metricsOn { + metrics.RecordTokenization(time.Since(startTime), len(input), err) + } return nil, err } @@ -716,8 +724,9 @@ func (t *Tokenizer) tokenize(ctx context.Context, input []byte, src string, have if tokenErr != nil { // Record metrics for failed tokenization - duration := time.Since(startTime) - metrics.RecordTokenization(duration, len(input), tokenErr) + if metricsOn { + metrics.RecordTokenization(time.Since(startTime), len(input), tokenErr) + } return nil, tokenErr } @@ -733,8 +742,9 @@ func (t *Tokenizer) tokenize(ctx context.Context, input []byte, src string, have } // Record metrics for successful tokenization - duration := time.Since(startTime) - metrics.RecordTokenization(duration, len(input), nil) + if metricsOn { + metrics.RecordTokenization(time.Since(startTime), len(input), nil) + } return tokens, nil } From f0ca0dc1f99fd8b82e301819a7cef3fafd30fe79 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:28:55 +0530 Subject: [PATCH 2/2] fix(tokenizer): correct metrics-gate comment and strengthen token-identity test [W1-02] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- pkg/sql/tokenizer/metrics_gate_test.go | 22 ++++++++++++++++++---- pkg/sql/tokenizer/tokenizer.go | 4 ++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/pkg/sql/tokenizer/metrics_gate_test.go b/pkg/sql/tokenizer/metrics_gate_test.go index 81abf1ae..61adacfd 100644 --- a/pkg/sql/tokenizer/metrics_gate_test.go +++ b/pkg/sql/tokenizer/metrics_gate_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/ajitpratap0/GoSQLX/pkg/metrics" + "github.com/ajitpratap0/GoSQLX/pkg/models" ) // TestTokenize_MetricsGate verifies tokenization is recorded when metrics are @@ -68,20 +69,33 @@ func TestTokenize_MetricsDoNotChangeTokens(t *testing.T) { } }) sql := []byte("SELECT a, COUNT(*) FROM t JOIN u ON t.id = u.id GROUP BY a") - get := func() int { + type tv struct { + typ models.TokenType + val string + } + get := func() []tv { tkz := GetTokenizer() defer PutTokenizer(tkz) toks, err := tkz.Tokenize(sql) if err != nil { t.Fatal(err) } - return len(toks) + out := make([]tv, len(toks)) + for i, tk := range toks { + out[i] = tv{tk.Token.Type, tk.Token.Value} + } + return out } metrics.Enable() a := get() metrics.Disable() b := get() - if a != b { - t.Fatalf("token count differs: on=%d off=%d", a, b) + if len(a) != len(b) { + t.Fatalf("token count differs: on=%d off=%d", len(a), len(b)) + } + for i := range a { + if a[i] != b[i] { + t.Fatalf("token %d differs: on=%v off=%v", i, a[i], b[i]) + } } } diff --git a/pkg/sql/tokenizer/tokenizer.go b/pkg/sql/tokenizer/tokenizer.go index cb439fbf..6d9e25b2 100644 --- a/pkg/sql/tokenizer/tokenizer.go +++ b/pkg/sql/tokenizer/tokenizer.go @@ -606,8 +606,8 @@ func (t *Tokenizer) load(input []byte, src string) { // in t.tokBuf and stays owned by t; see tokBuf. func (t *Tokenizer) tokenize(ctx context.Context, input []byte, src string, haveSrc, scratch bool) ([]models.TokenWithSpan, error) { // Read the clock only when metrics are enabled; time.Now is a measurable - // share of short tokenizations. If metrics are toggled on mid-call the - // duration is recorded as 0. + // share of short tokenizations. If metrics are enabled mid-call, that call + // is not recorded. var startTime time.Time metricsOn := metrics.IsEnabled() if metricsOn {