diff --git a/CHANGELOG.md b/CHANGELOG.md index b3801161..df4d1b49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 are removed. ### 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..61adacfd --- /dev/null +++ b/pkg/sql/tokenizer/metrics_gate_test.go @@ -0,0 +1,101 @@ +// Copyright 2026 GoSQLX Contributors +// SPDX-License-Identifier: Apache-2.0 + +package tokenizer + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/metrics" + "github.com/ajitpratap0/GoSQLX/pkg/models" +) + +// 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") + 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) + } + 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 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 745b90a2..6d9e25b2 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 enabled mid-call, that call + // is not recorded. + 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 }