From c84cde61750e4bb73ba44f14c599d68b3c41b0bb Mon Sep 17 00:00:00 2001 From: CharlesKWON Date: Sat, 18 Apr 2026 13:40:19 +0900 Subject: [PATCH] =?UTF-8?q?perf(fivesql2):=20Go-native=20SqlIsAggName=20?= =?UTF-8?q?=E2=80=94=20drop=20per-row=20substring=20scan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B4 GROUP+HAVING profile showed SqlIsAggName at ~9% of CPU — SqlEvalFunc checks it for every function in every row, and the PRG body was two string allocations + a substring scan: RETURN ("," + c + ",") $ ("," + AGG_FUNCTIONS + ",") Replace with a hash lookup against the existing aggFuncSet map in hbrtl/sqlexpr.go (already populated for SqlExprHasAgg, same AGG_FUNCTIONS list). Upper-casing skips the allocation when the input is already upper, which it almost always is in practice. Bench deltas (median of 3 steady runs, 1000 iters): B4_GROUP_HAVING 447 → 418 us -6.5% B14_COUNT 252 → 235 us -7% B15_CTE_WIN_JOIN 1595 → 1577 us -1% Other benches unchanged (no aggregate calls per row). FiveSql2 43/43, Harbour compat 56/56. Co-Authored-By: Claude Opus 4.7 (1M context) --- _FiveSql2/src/TSqlExpr.prg | 9 ++++++--- hbrtl/register.go | 1 + hbrtl/sqlhelpers.go | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/_FiveSql2/src/TSqlExpr.prg b/_FiveSql2/src/TSqlExpr.prg index 0cd78e3..14adfd0 100644 --- a/_FiveSql2/src/TSqlExpr.prg +++ b/_FiveSql2/src/TSqlExpr.prg @@ -47,9 +47,12 @@ RETURN "expr" * to avoid a name collision with the RTL symbol; behavior is * byte-for-byte identical. See docs/RTL-Go-Native-Migration.md. */ -/* Return .T. if the function name is an aggregate */ -FUNCTION SqlIsAggName( c ) -RETURN ( "," + c + "," ) $ ( "," + AGG_FUNCTIONS + "," ) +/* SqlIsAggName is implemented in Go (hbrtl/sqlhelpers.go) — registered + * as SQLISAGGNAME. Former PRG body: + * RETURN ( "," + c + "," ) $ ( "," + AGG_FUNCTIONS + "," ) + * Removed to avoid the symbol collision; Go version uses a hash lookup + * against aggFuncSet (same AGG_FUNCTIONS list) and skips the substring + * scan + string allocations that were ~9% of GROUP BY CPU. */ /* Return .T. if the function name is a recognized scalar */ FUNCTION SqlIsScalarName( c ) diff --git a/hbrtl/register.go b/hbrtl/register.go index 398d9e4..bea4ed0 100644 --- a/hbrtl/register.go +++ b/hbrtl/register.go @@ -637,6 +637,7 @@ func RegisterRTL(vm *hbrt.VM) { hbrt.Sym("SQLCOERCENUM", hbrt.FsPublic, SqlCoerceNum), hbrt.Sym("SQLCOERCEFORCMP", hbrt.FsPublic, SqlCoerceForCmp), hbrt.Sym("SQLISTRUE", hbrt.FsPublic, SqlIsTrue), + hbrt.Sym("SQLISAGGNAME", hbrt.FsPublic, SqlIsAggName), hbrt.Sym("SQLCMPEQ", hbrt.FsPublic, SqlCmpEq), hbrt.Sym("SQLCMPLT", hbrt.FsPublic, SqlCmpLt), hbrt.Sym("SQLEXTRACTTEMPLATE", hbrt.FsPublic, SqlExtractTemplate), diff --git a/hbrtl/sqlhelpers.go b/hbrtl/sqlhelpers.go index e8d0723..76a075e 100644 --- a/hbrtl/sqlhelpers.go +++ b/hbrtl/sqlhelpers.go @@ -585,3 +585,30 @@ func sqlCmpLt(a, b hbrt.Value) bool { } return false } + +// SqlIsAggName(cName) → lBool +// Go-native replacement for TSqlExpr.prg SqlIsAggName. The PRG version +// was `("," + c + ",") $ ("," + AGG_FUNCTIONS + ",")` — two string +// allocations + a substring scan per call. Profile showed this at +// 8.7% of B4 GROUP+HAVING CPU. Uses the aggFuncSet already declared +// in sqlexpr.go for SqlExprHasAgg. +func SqlIsAggName(t *hbrt.Thread) { + t.Frame(1, 0) + defer t.EndProc() + name := t.Local(1).AsString() + if name == "" { + t.RetBool(false) + return + } + // Upper-case without allocating unless needed. + upper := name + for i := 0; i < len(name); i++ { + c := name[i] + if c >= 'a' && c <= 'z' { + upper = strings.ToUpper(name) + break + } + } + _, ok := aggFuncSet[upper] + t.RetBool(ok) +}