From d410686c63e402bc159557544e0ba217f8e393d9 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:12:43 +0800 Subject: [PATCH 1/4] perf: make script parsing linear in input length Parsed output was accumulated with string +=, one rune at a time, in parseArgs, parseAdvArgs, parseQuotedArg, parseCommand and every function in expressions.go. Go strings are immutable, so each += allocated a new string and copied everything accumulated so far: an N-character argument cost N^2/2 bytes copied and two allocations per character. Parsing "**launch:" followed by 64KB of text took 236ms and allocated 2.3GB. Accumulate through strings.Builder instead, matching parseJSONArg and parseMediaTitleSyntax which already did. Parse time is now 7.3ns per byte, flat from 888 bytes to 1MB, and the same 64KB argument takes 485us and allocates 286KB. Read the input in place rather than through a bufio.Reader over a copy of it. The input is already a string in memory, so buffering it copied the whole script and allocated a 4KB buffer for every parser, which is dead weight when a caller parses the same short script several times. peek still invalidates a pending unread, as the buffered reader did, so the two cannot start being relied on to compose. Return the input unchanged from ParseExpressions and EvalExpressions when it holds no expression opener. Every argument of every command is evaluated on the launch path and almost none carry an expression; evaluating one that does not now costs 22ns and no allocation, against 2357ns and 94 allocations. --- arguments.go | 71 +++++++++++++++++++--------------- expressions.go | 102 +++++++++++++++++++++++++++++-------------------- parser.go | 19 +++++---- reader.go | 99 +++++++++++++++++++++++++++++++---------------- 4 files changed, 180 insertions(+), 111 deletions(-) diff --git a/arguments.go b/arguments.go index 22f67be..0ece129 100644 --- a/arguments.go +++ b/arguments.go @@ -428,18 +428,19 @@ func expandTokenN(token string, n int, totalLen *int) ([]string, error) { func (sr *ScriptReader) parseAdvArgs() (advArgs map[string]string, remainingStr string, err error) { advArgs = make(map[string]string) inValue := false - currentArg := "" - currentValue := "" + // Builders for the same reason as parseArgs: an advanced argument value + // can be as long as the rest of the command, and rune-at-a-time string + // concatenation makes that quadratic. + var currentArg, currentValue strings.Builder valueStart := int64(-1) buf := make([]rune, 0, 64) storeArg := func() { - if currentArg != "" { - currentValue = strings.TrimSpace(currentValue) - advArgs[currentArg] = currentValue + if currentArg.Len() > 0 { + advArgs[currentArg.String()] = strings.TrimSpace(currentValue.String()) } - currentArg = "" - currentValue = "" + currentArg.Reset() + currentValue.Reset() } for { @@ -459,14 +460,16 @@ func (sr *ScriptReader) parseAdvArgs() (advArgs map[string]string, remainingStr if parseErr != nil { return advArgs, string(buf), parseErr } - currentValue = quotedValue + currentValue.Reset() + _, _ = currentValue.WriteString(quotedValue) continue case ch == SymJSONStart && valueStart == sr.pos-1: jsonValue, parseErr := sr.parseJSONArg() if parseErr != nil { return advArgs, string(buf), parseErr } - currentValue = jsonValue + currentValue.Reset() + _, _ = currentValue.WriteString(jsonValue) continue case ch == SymEscapeSeq: // Peek next char for raw tracking before parseEscapeSeq consumes it @@ -479,11 +482,11 @@ func (sr *ScriptReader) parseAdvArgs() (advArgs map[string]string, remainingStr if escapeErr != nil { return advArgs, string(buf), escapeErr } else if next == "" { - currentValue += string(SymEscapeSeq) + _, _ = currentValue.WriteRune(SymEscapeSeq) continue } buf = append(buf, nextRaw) - currentValue += next + _, _ = currentValue.WriteString(next) continue } } @@ -512,15 +515,15 @@ func (sr *ScriptReader) parseAdvArgs() (advArgs map[string]string, remainingStr if err != nil { return advArgs, string(buf), err } - currentValue += exprValue + _, _ = currentValue.WriteString(exprValue) } else { - currentValue += string(ch) + _, _ = currentValue.WriteRune(ch) } continue case !isAdvArgName(ch): return advArgs, string(buf), ErrInvalidAdvArgName default: - currentArg += string(ch) + _, _ = currentArg.WriteRune(ch) } } @@ -536,7 +539,12 @@ func (sr *ScriptReader) parseArgs( ) (args []string, advArgs map[string]string, err error) { args = make([]string, 0) advArgs = make(map[string]string) - currentArg := prefix + // currentArg accumulates through a Builder rather than string + // concatenation: appending a rune at a time to a string reallocates and + // copies the whole argument on every character, which is quadratic in + // the length of the argument. + var currentArg strings.Builder + _, _ = currentArg.WriteString(prefix) argStart := sr.pos // tracks whether content was explicitly written, distinguishing // "**cmd:" (no content, no arg) from "**cmd:''" (explicit empty arg) @@ -557,7 +565,9 @@ argsLoop: if quotedErr != nil { return args, advArgs, quotedErr } - currentArg = quotedArg + // a quoted argument replaces whatever preceded it + currentArg.Reset() + _, _ = currentArg.WriteString(quotedArg) argWritten = true continue argsLoop case argStart == sr.pos-1 && ch == SymJSONStart: @@ -565,7 +575,8 @@ argsLoop: if jsonErr != nil { return args, advArgs, jsonErr } - currentArg = jsonArg + currentArg.Reset() + _, _ = currentArg.WriteString(jsonArg) argWritten = true continue argsLoop case ch == SymEscapeSeq: @@ -574,11 +585,11 @@ argsLoop: if escapeErr != nil { return args, advArgs, escapeErr } else if next == "" { - currentArg += string(SymEscapeSeq) + _, _ = currentArg.WriteRune(SymEscapeSeq) argWritten = true continue argsLoop } - currentArg += next + _, _ = currentArg.WriteString(next) argWritten = true continue argsLoop } @@ -593,9 +604,8 @@ argsLoop: switch { case !onlyOneArg && ch == SymArgSep: // new argument - currentArg = strings.TrimSpace(currentArg) - args = append(args, currentArg) - currentArg = "" + args = append(args, strings.TrimSpace(currentArg.String())) + currentArg.Reset() argStart = sr.pos argWritten = false continue argsLoop @@ -605,7 +615,8 @@ argsLoop: case errors.Is(err, ErrInvalidAdvArgName): // if an adv arg name is invalid, fallback on treating it // as a positional arg with a ? in it - currentArg += string(SymAdvArgStart) + buf + _, _ = currentArg.WriteRune(SymAdvArgStart) + _, _ = currentArg.WriteString(buf) continue argsLoop case err != nil: return args, advArgs, err @@ -620,11 +631,11 @@ argsLoop: if err != nil { return args, advArgs, err } - currentArg += exprValue + _, _ = currentArg.WriteString(exprValue) argWritten = true continue argsLoop default: - currentArg += string(ch) + _, _ = currentArg.WriteRune(ch) if !isWhitespace(ch) { argWritten = true } @@ -632,12 +643,12 @@ argsLoop: } } - currentArg = strings.TrimSpace(currentArg) - if !onlyAdvArgs && (currentArg != "" || argWritten) { - args = append(args, currentArg) - } else if onlyAdvArgs && currentArg != "" { + finalArg := strings.TrimSpace(currentArg.String()) + if !onlyAdvArgs && (finalArg != "" || argWritten) { + args = append(args, finalArg) + } else if onlyAdvArgs && finalArg != "" { // fallback content from invalid adv args should still be preserved - args = append(args, currentArg) + args = append(args, finalArg) } return args, advArgs, nil diff --git a/expressions.go b/expressions.go index 5aabcac..bb1085e 100644 --- a/expressions.go +++ b/expressions.go @@ -98,15 +98,16 @@ type CustomLauncherExprEnv struct { } func (sr *ScriptReader) parseExpression() (string, error) { - rawExpr := TokExpStart + var rawExpr strings.Builder + _, _ = rawExpr.WriteString(TokExpStart) next, err := sr.read() if err != nil { - return rawExpr, err + return rawExpr.String(), err } else if next != SymExpressionStart { err := sr.unread() if err != nil { - return rawExpr, err + return rawExpr.String(), err } return string(SymExpressionStart), nil } @@ -114,51 +115,51 @@ func (sr *ScriptReader) parseExpression() (string, error) { for { ch, err := sr.read() if err != nil { - return rawExpr, err + return rawExpr.String(), err } else if ch == eof { - return rawExpr, ErrUnmatchedExpression + return rawExpr.String(), ErrUnmatchedExpression } if ch == SymExpressionEnd { next, err := sr.peek() if err != nil { - return rawExpr, err + return rawExpr.String(), err } else if next == SymExpressionEnd { - rawExpr += TokExprEnd + _, _ = rawExpr.WriteString(TokExprEnd) err := sr.skip() if err != nil { - return rawExpr, err + return rawExpr.String(), err } break } } - rawExpr += string(ch) + _, _ = rawExpr.WriteRune(ch) } - return rawExpr, nil + return rawExpr.String(), nil } func (sr *ScriptReader) parsePostExpression() (string, error) { - rawExpr := "" + var rawExpr strings.Builder exprEndToken, _ := utf8.DecodeRuneInString(TokExprEnd) for { ch, err := sr.read() if err != nil { - return rawExpr, err + return rawExpr.String(), err } else if ch == eof { - return rawExpr, ErrUnmatchedExpression + return rawExpr.String(), ErrUnmatchedExpression } if ch == exprEndToken { break } - rawExpr += string(ch) + _, _ = rawExpr.WriteRune(ch) } - return rawExpr, nil + return rawExpr.String(), nil } // ParseExpressions parses and converts expressions in the input string from @@ -166,12 +167,21 @@ func (sr *ScriptReader) parsePostExpression() (string, error) { // to be evaluated by the EvalExpressions function. This function ONLY parses // expression symbols and escape sequences, no other ZapScript syntax. func (sr *ScriptReader) ParseExpressions() (string, error) { - result := "" + // Without an escape sequence or an expression opener there is nothing to + // rewrite and the output is the input. Almost every argument on a launch + // path is in that shape, so it is worth not walking it twice. + if rest := sr.remaining(); !strings.ContainsRune(rest, SymEscapeSeq) && + !strings.ContainsRune(rest, SymExpressionStart) { + sr.consumeAll() + return rest, nil + } + + var result strings.Builder for { ch, err := sr.read() if err != nil { - return result, err + return result.String(), err } else if ch == eof { break } @@ -180,32 +190,50 @@ func (sr *ScriptReader) ParseExpressions() (string, error) { case SymEscapeSeq: next, err := sr.parseEscapeSeq() if err != nil { - return result, err + return result.String(), err } - result += next + _, _ = result.WriteString(next) continue case SymExpressionStart: exprValue, err := sr.parseExpression() if err != nil { - return result, err + return result.String(), err } - result += exprValue + _, _ = result.WriteString(exprValue) continue default: - result += string(ch) + _, _ = result.WriteRune(ch) continue } } - return result, nil + return result.String(), nil } func (sr *ScriptReader) EvalExpressions(exprEnv any) (string, error) { - parts := make([]PostArgPart, 0) - currentPart := PostArgPart{} - exprStartToken, _ := utf8.DecodeRuneInString(TokExpStart) + // An argument with no expression in it evaluates to itself. Every + // argument of every command goes through here on the launch path and + // almost none of them carry an expression. + if rest := sr.remaining(); !strings.ContainsRune(rest, exprStartToken) { + sr.consumeAll() + return rest, nil + } + + parts := make([]PostArgPart, 0) + var pending strings.Builder + + flushPending := func() { + if pending.Len() > 0 { + parts = append(parts, PostArgPart{ + Value: pending.String(), + Type: ArgPartTypeString, + }) + pending.Reset() + } + } + for { ch, err := sr.read() if err != nil { @@ -215,31 +243,23 @@ func (sr *ScriptReader) EvalExpressions(exprEnv any) (string, error) { } if ch == exprStartToken { - if currentPart.Type != ArgPartTypeUnknown { - parts = append(parts, currentPart) - currentPart = PostArgPart{} - } + flushPending() - currentPart.Type = ArgPartTypeExpression exprValue, err := sr.parsePostExpression() if err != nil { return "", err } - currentPart.Value = exprValue - - parts = append(parts, currentPart) - currentPart = PostArgPart{} + parts = append(parts, PostArgPart{ + Value: exprValue, + Type: ArgPartTypeExpression, + }) continue } - currentPart.Type = ArgPartTypeString - currentPart.Value += string(ch) - continue + _, _ = pending.WriteRune(ch) } - if currentPart.Type != ArgPartTypeUnknown { - parts = append(parts, currentPart) - } + flushPending() var result strings.Builder for _, part := range parts { diff --git a/parser.go b/parser.go index e551e44..51be5b0 100644 --- a/parser.go +++ b/parser.go @@ -106,7 +106,7 @@ func (sr *ScriptReader) parseMediaTitleSyntax() (*mediaTitleParseResult, error) break } - _, _ = contentBuilder.WriteString(string(ch)) + _, _ = contentBuilder.WriteRune(ch) } rawContent += contentBuilder.String() @@ -136,6 +136,10 @@ func (sr *ScriptReader) parseMediaTitleSyntax() (*mediaTitleParseResult, error) func (sr *ScriptReader) parseCommand(onlyOneArg bool) (Command, string, error) { cmd := Command{} var buf []rune + // A command name is short in practice, but nothing bounds it: the + // grammar accepts name characters until it meets a separator, so an + // unterminated name is as long as the input. + var name strings.Builder commandLoop: for { @@ -157,14 +161,14 @@ commandLoop: switch { case isCmdName(ch): - cmd.Name += string(ch) + _, _ = name.WriteRune(ch) case ch == SymArgStart || ch == SymAdvArgStart: // parse arguments - if cmd.Name == "" { + if name.Len() == 0 { break commandLoop } - cmd.Name = normalizeCmdName(cmd.Name) + cmd.Name = normalizeCmdName(name.String()) onlyAdvArgs := false if ch == SymAdvArgStart { @@ -214,11 +218,12 @@ commandLoop: } if cmd.Name == "" { - return cmd, string(buf), ErrEmptyCmdName + if name.Len() == 0 { + return cmd, string(buf), ErrEmptyCmdName + } + cmd.Name = normalizeCmdName(name.String()) } - cmd.Name = normalizeCmdName(cmd.Name) - return cmd, string(buf), nil } diff --git a/reader.go b/reader.go index 3f76b35..9389080 100644 --- a/reader.go +++ b/reader.go @@ -16,12 +16,9 @@ package zapscript import ( - "bufio" - "bytes" "encoding/json" "errors" "fmt" - "io" "sort" "strings" "unicode/utf8" @@ -258,49 +255,85 @@ type mediaTitleParseResult struct { valid bool } +var ( + errUnreadNotPossible = errors.New("no rune available to unread") + errRuneError = errors.New("rune error") +) + +// ScriptReader walks the input one rune at a time, forward only. +// +// The input is held as the caller's string and decoded in place. Reading +// through a buffer would copy the whole script and allocate a buffer for +// every parser, which is dead weight for input that is already in memory and +// is measurable when a caller parses the same short script several times. type ScriptReader struct { - r *bufio.Reader + src string + // off is the byte offset of the next rune to read. + off int + // lastWidth is the byte width of the rune returned by the most recent + // read, or 0 when there is nothing to unread. + lastWidth int + // pos counts runes consumed. Argument parsing compares it against a + // saved position to tell whether a quote or brace opened an argument, so + // it counts runes rather than bytes. pos int64 } func NewParser(value string) *ScriptReader { - return &ScriptReader{ - r: bufio.NewReader(bytes.NewReader([]byte(value))), - } + return &ScriptReader{src: value} } func (sr *ScriptReader) read() (rune, error) { - ch, _, err := sr.r.ReadRune() - if errors.Is(err, io.EOF) { + if sr.off >= len(sr.src) { + sr.lastWidth = 0 return eof, nil - } else if err != nil { - return eof, fmt.Errorf("failed to read rune: %w", err) } + ch, width := utf8.DecodeRuneInString(sr.src[sr.off:]) + sr.off += width + sr.lastWidth = width sr.pos++ return ch, nil } +// unread steps back over the rune returned by the most recent read. Only that +// rune can be returned: a second unread, or one after a peek or at end of +// input, is a bug in the caller rather than a condition to recover from. func (sr *ScriptReader) unread() error { - err := sr.r.UnreadRune() - if err != nil { - return fmt.Errorf("failed to unread rune: %w", err) + if sr.lastWidth == 0 { + return errUnreadNotPossible } + sr.off -= sr.lastWidth + sr.lastWidth = 0 sr.pos-- return nil } +// remaining returns the text the reader has not consumed yet. It is a slice +// of the original input, not a copy. +func (sr *ScriptReader) remaining() string { + return sr.src[sr.off:] +} + +// consumeAll advances the reader to the end of the input, for the fast paths +// that return the remaining text verbatim instead of walking it. +func (sr *ScriptReader) consumeAll() { + sr.pos += int64(utf8.RuneCountInString(sr.remaining())) + sr.off = len(sr.src) + sr.lastWidth = 0 +} + func (sr *ScriptReader) peek() (rune, error) { - for peekBytes := 4; peekBytes > 0; peekBytes-- { - b, err := sr.r.Peek(peekBytes) - if err == nil { - r, _ := utf8.DecodeRune(b) - if r == utf8.RuneError { - return r, errors.New("rune error") - } - return r, nil - } + // A peek invalidates the pending unread, matching the buffered reader + // this replaced, so a caller cannot start relying on the two composing. + sr.lastWidth = 0 + if sr.off >= len(sr.src) { + return eof, nil + } + r, _ := utf8.DecodeRuneInString(sr.src[sr.off:]) + if r == utf8.RuneError { + return r, errRuneError } - return eof, nil + return r, nil } func (sr *ScriptReader) skip() error { @@ -361,29 +394,29 @@ func (sr *ScriptReader) parseEscapeSeq() (string, error) { } func (sr *ScriptReader) parseQuotedArg(start rune) (string, error) { - arg := "" + var arg strings.Builder for { ch, err := sr.read() if err != nil { - return arg, err + return arg.String(), err } else if ch == eof { - return arg, ErrUnmatchedQuote + return arg.String(), ErrUnmatchedQuote } if ch == SymEscapeSeq { next, err := sr.parseEscapeSeq() if err != nil { - return arg, err + return arg.String(), err } - arg += next + _, _ = arg.WriteString(next) continue } else if ch == SymExpressionStart { exprValue, err := sr.parseExpression() if err != nil { - return arg, err + return arg.String(), err } - arg += exprValue + _, _ = arg.WriteString(exprValue) continue } @@ -391,8 +424,8 @@ func (sr *ScriptReader) parseQuotedArg(start rune) (string, error) { break } - arg += string(ch) + _, _ = arg.WriteRune(ch) } - return arg, nil + return arg.String(), nil } From e37991e017a08d8bc5f110fb2b3932a95cd36769 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:12:43 +0800 Subject: [PATCH 2/4] test: add parser benchmarks and a per-character allocation guard There were no benchmarks in the repository, so the quadratic accumulator went unnoticed for as long as it did. BenchmarkParseScript_LongArg and its siblings sweep argument length so non-linear growth is visible, and BenchmarkParseScript_TypicalToken covers the shapes that run on a tap, where fixed per-parse costs dominate instead. TestParseScript_LongInputDoesNotAllocatePerCharacter is the guard that actually fails on a regression: rune-at-a-time accumulation allocated 32,785 times for a 16KB argument, against a ceiling of 200. It does not run in parallel because testing.AllocsPerRun pins GOMAXPROCS. --- Taskfile.yml | 8 +++ parser_alloc_test.go | 112 +++++++++++++++++++++++++++++++++++++++ parser_bench_test.go | 123 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+) create mode 100644 parser_alloc_test.go create mode 100644 parser_bench_test.go diff --git a/Taskfile.yml b/Taskfile.yml index 770a45f..41be2e4 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -16,6 +16,14 @@ tasks: cmds: - go test -race ./... + bench: + desc: Run benchmarks (use BENCH to filter, COUNT to repeat) + vars: + BENCH: '{{default "." .BENCH}}' + COUNT: '{{default "1" .COUNT}}' + cmds: + - go test -run='^$' -bench='{{.BENCH}}' -benchmem -count={{.COUNT}} ./... + fuzz: desc: Run fuzz tests (default 30s per test, use FUZZ_TIME to override) vars: diff --git a/parser_alloc_test.go b/parser_alloc_test.go new file mode 100644 index 0000000..f56f002 --- /dev/null +++ b/parser_alloc_test.go @@ -0,0 +1,112 @@ +// Copyright 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package zapscript_test + +import ( + "strings" + "testing" + + zapscript "github.com/ZaparooProject/go-zapscript" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestParseScript_LongInputDoesNotAllocatePerCharacter guards the property +// that makes parse time linear. +// +// Accumulating a rune at a time into a string reallocates and copies the +// whole accumulator on every character, which is two allocations per +// character and quadratic total copying. Growing a strings.Builder instead is +// logarithmic in the length. At this input size the two differ by more than +// three orders of magnitude, so the ceiling below is generous and still fails +// loudly if an accumulator regresses. +// +// This test does not call t.Parallel: testing.AllocsPerRun pins GOMAXPROCS +// for the duration and must not run alongside other tests. +func TestParseScript_LongInputDoesNotAllocatePerCharacter(t *testing.T) { + const ( + size = 16384 + // Per-character accumulation allocated ~32,800 times at this size. + maxAllocs = 200.0 + ) + + arg := strings.Repeat("A", size) + + tests := []struct { + name string + script string + }{ + {name: "positional arg", script: "**launch:" + arg}, + {name: "quoted arg", script: `**launch:"` + arg + `"`}, + {name: "advanced arg value", script: "**launch:game?name=" + arg}, + {name: "command name", script: "**" + strings.ToLower(arg)}, + {name: "auto launch content", script: arg}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + allocs := testing.AllocsPerRun(2, func() { + //nolint:errcheck // an unknown command name still parses; only allocation is under test + _, _ = zapscript.NewParser(tt.script).ParseScript() + }) + assert.Less(t, allocs, maxAllocs, + "parse allocated %.0f times for %d characters, which is per-character accumulation", + allocs, size) + }) + } +} + +// A long argument has to survive the parse intact, not merely parse quickly. +func TestParseScript_LongArgumentRoundTrips(t *testing.T) { + t.Parallel() + + arg := strings.Repeat("A", 16384) + + tests := []struct { + name string + script string + want string + }{ + {name: "positional arg", script: "**launch:" + arg, want: arg}, + {name: "quoted arg", script: `**launch:"` + arg + `"`, want: arg}, + {name: "auto launch content", script: arg, want: arg}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + script, err := zapscript.NewParser(tt.script).ParseScript() + require.NoError(t, err) + require.Len(t, script.Cmds, 1) + require.Len(t, script.Cmds[0].Args, 1) + assert.Equal(t, tt.want, script.Cmds[0].Args[0]) + }) + } +} + +// The advanced-argument value accumulator is separate from the positional +// one, so it needs its own round trip. +func TestParseScript_LongAdvArgRoundTrips(t *testing.T) { + t.Parallel() + + value := strings.Repeat("A", 16384) + + script, err := zapscript.NewParser("**launch:game?name=" + value).ParseScript() + require.NoError(t, err) + require.Len(t, script.Cmds, 1) + assert.Equal(t, value, script.Cmds[0].AdvArgs.Get("name")) +} diff --git a/parser_bench_test.go b/parser_bench_test.go new file mode 100644 index 0000000..73c2fee --- /dev/null +++ b/parser_bench_test.go @@ -0,0 +1,123 @@ +// Copyright 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package zapscript_test + +import ( + "fmt" + "strings" + "testing" + + zapscript "github.com/ZaparooProject/go-zapscript" +) + +// benchArgLengths span the range a caller can realistically hand the parser. +// Parse cost must stay proportional to length: a doubling that costs four +// times as much is the quadratic accumulator regressing. +var benchArgLengths = []int{1024, 4096, 16384, 65536} + +func BenchmarkParseScript_LongArg(b *testing.B) { + for _, size := range benchArgLengths { + script := "**launch:" + strings.Repeat("A", size) + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if _, err := zapscript.NewParser(script).ParseScript(); err != nil { + b.Fatal(err) + } + } + }) + } +} + +func BenchmarkParseScript_LongQuotedArg(b *testing.B) { + for _, size := range benchArgLengths { + script := `**launch:"` + strings.Repeat("A", size) + `"` + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if _, err := zapscript.NewParser(script).ParseScript(); err != nil { + b.Fatal(err) + } + } + }) + } +} + +func BenchmarkParseScript_LongAdvArg(b *testing.B) { + for _, size := range benchArgLengths { + script := "**launch:game?name=" + strings.Repeat("A", size) + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if _, err := zapscript.NewParser(script).ParseScript(); err != nil { + b.Fatal(err) + } + } + }) + } +} + +// BenchmarkParseScript_LongCmdName covers the command-name accumulator, which +// is reached before argument parsing starts. +func BenchmarkParseScript_LongCmdName(b *testing.B) { + for _, size := range benchArgLengths { + script := "**" + strings.Repeat("a", size) + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + // An unknown command name still parses; only the name + // accumulator is being measured here. + _, _ = zapscript.NewParser(script).ParseScript() + } + }) + } +} + +// BenchmarkParseScript_TypicalToken is the shape that actually runs on a tap. +// It is dominated by per-parse fixed costs rather than argument length. +func BenchmarkParseScript_TypicalToken(b *testing.B) { + scripts := map[string]string{ + "launch_path": "**launch:/media/fat/games/SNES/Super Metroid (USA).sfc", + "media_title": "@SNES/Super Metroid", + "bare_path": "/media/fat/games/SNES/Super Metroid (USA).sfc", + "command_chain": "**launch.system:snes||**delay:500||**input.keyboard:{f12}", + "adv_args": "**launch.search:mario?system=snes&launcher=retroarch", + } + + for name, script := range scripts { + b.Run(name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if _, err := zapscript.NewParser(script).ParseScript(); err != nil { + b.Fatal(err) + } + } + }) + } +} + +// BenchmarkEvalExpressions_NoExpression is the common case on the launch +// path: every argument of every command is evaluated, and almost none of them +// contain an expression. +func BenchmarkEvalExpressions_NoExpression(b *testing.B) { + arg := "/media/fat/games/SNES/Super Metroid (USA).sfc" + b.ReportAllocs() + for b.Loop() { + if _, err := zapscript.NewParser(arg).EvalExpressions(zapscript.ArgExprEnv{}); err != nil { + b.Fatal(err) + } + } +} From 93a1da8a4956017d1268fc9a168a2e6d8bf1fa91 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:30:07 +0800 Subject: [PATCH 3/4] fix: treat a literal U+FFFD in the input as content peek reported any rune that decoded to U+FFFD as a rune error, but DecodeRuneInString returns that rune for two different things: invalid encoding, which is one byte wide, and a replacement character actually written in the input, which is three. The parser peeks after a command separator, an expression terminator and a command prefix, so a script such as "**launch:a|" failed to parse. Only treat the one-byte form as an encoding error. This predates the reader rewrite in this branch and behaves the same on v0.18.0. --- reader.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/reader.go b/reader.go index 9389080..a3bdc40 100644 --- a/reader.go +++ b/reader.go @@ -329,8 +329,11 @@ func (sr *ScriptReader) peek() (rune, error) { if sr.off >= len(sr.src) { return eof, nil } - r, _ := utf8.DecodeRuneInString(sr.src[sr.off:]) - if r == utf8.RuneError { + r, width := utf8.DecodeRuneInString(sr.src[sr.off:]) + // DecodeRuneInString reports invalid encoding as U+FFFD with a width of + // one. A U+FFFD that was actually written in the input decodes to the + // same rune but is three bytes wide, and is ordinary content. + if r == utf8.RuneError && width <= 1 { return r, errRuneError } return r, nil From 2e55d0a518f4518bb6fc763d2c276df978cd3586 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:30:07 +0800 Subject: [PATCH 4/4] test: round trip long command names, media titles and multibyte input The allocation guard proves parsing does not scale with input length but says nothing about what the parse produced. Extend the round trips to the accumulators it did not reach: the command name, the media title, and advanced argument values. Cover multibyte and escaped content as well. The reader decodes runes from the input in place now, so a rune spanning several bytes exercises the offset arithmetic that replaced the buffered reader. Compare with cmp.Diff, as the other parser tests do. TestParseScript_LiteralReplacementCharacterIsContent fails without the peek fix in the preceding commit. --- parser_alloc_test.go | 104 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/parser_alloc_test.go b/parser_alloc_test.go index f56f002..f1ca9af 100644 --- a/parser_alloc_test.go +++ b/parser_alloc_test.go @@ -20,6 +20,7 @@ import ( "testing" zapscript "github.com/ZaparooProject/go-zapscript" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -70,19 +71,35 @@ func TestParseScript_LongInputDoesNotAllocatePerCharacter(t *testing.T) { } // A long argument has to survive the parse intact, not merely parse quickly. +// Multibyte and escaped content is covered too: the reader decodes runes from +// the input in place, so a rune that spans several bytes exercises the offset +// arithmetic that replaced the buffered reader. func TestParseScript_LongArgumentRoundTrips(t *testing.T) { t.Parallel() - arg := strings.Repeat("A", 16384) + const reps = 4096 + ascii := strings.Repeat("A", 16384) + multibyte := strings.Repeat("\u3042", reps) tests := []struct { name string script string want string }{ - {name: "positional arg", script: "**launch:" + arg, want: arg}, - {name: "quoted arg", script: `**launch:"` + arg + `"`, want: arg}, - {name: "auto launch content", script: arg, want: arg}, + {name: "positional arg", script: "**launch:" + ascii, want: ascii}, + {name: "quoted arg", script: `**launch:"` + ascii + `"`, want: ascii}, + {name: "auto launch content", script: ascii, want: ascii}, + {name: "multibyte positional arg", script: "**launch:" + multibyte, want: multibyte}, + { + name: "multibyte quoted arg", + script: `**launch:"` + multibyte + `"`, + want: multibyte, + }, + { + name: "escape sequences", + script: "**launch:" + strings.Repeat("a^nb", reps), + want: strings.Repeat("a\nb", reps), + }, } for _, tt := range tests { @@ -93,7 +110,9 @@ func TestParseScript_LongArgumentRoundTrips(t *testing.T) { require.NoError(t, err) require.Len(t, script.Cmds, 1) require.Len(t, script.Cmds[0].Args, 1) - assert.Equal(t, tt.want, script.Cmds[0].Args[0]) + if diff := cmp.Diff(tt.want, script.Cmds[0].Args[0]); diff != "" { + t.Errorf("argument mismatch (-want +got):\n%s", diff) + } }) } } @@ -108,5 +127,78 @@ func TestParseScript_LongAdvArgRoundTrips(t *testing.T) { script, err := zapscript.NewParser("**launch:game?name=" + value).ParseScript() require.NoError(t, err) require.Len(t, script.Cmds, 1) - assert.Equal(t, value, script.Cmds[0].AdvArgs.Get("name")) + if diff := cmp.Diff(value, script.Cmds[0].AdvArgs.Get("name")); diff != "" { + t.Errorf("advanced argument mismatch (-want +got):\n%s", diff) + } +} + +// The command name and the media title each accumulate through their own +// builder, so neither is covered by the argument round trips above. +func TestParseScript_LongCommandNameRoundTrips(t *testing.T) { + t.Parallel() + + name := strings.Repeat("a", 16384) + + script, err := zapscript.NewParser("**" + name + ":arg").ParseScript() + require.NoError(t, err) + require.Len(t, script.Cmds, 1) + if diff := cmp.Diff(name, script.Cmds[0].Name); diff != "" { + t.Errorf("command name mismatch (-want +got):\n%s", diff) + } +} + +func TestParseScript_LongMediaTitleRoundTrips(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + title string + }{ + {name: "ascii", title: strings.Repeat("A", 16384)}, + {name: "multibyte", title: strings.Repeat("\u3042", 4096)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + script, err := zapscript.NewParser("@system/" + tt.title).ParseScript() + require.NoError(t, err) + require.Len(t, script.Cmds, 1) + require.Len(t, script.Cmds[0].Args, 1) + if diff := cmp.Diff("system/"+tt.title, script.Cmds[0].Args[0]); diff != "" { + t.Errorf("media title mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// peek reports invalid encoding as a rune error. A U+FFFD actually present in +// the input is ordinary content and must not be mistaken for one, which it +// was before: the parser only peeks after a command separator, an expression +// terminator or a command prefix, so this reached a caller as a parse error. +func TestParseScript_LiteralReplacementCharacterIsContent(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + script string + want string + }{ + {name: "after a command separator", script: "**launch:a|\ufffd", want: "a|\ufffd"}, + {name: "as the whole argument", script: "**launch:\ufffd", want: "\ufffd"}, + {name: "mid argument", script: "**launch:a\ufffdb", want: "a\ufffdb"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + script, err := zapscript.NewParser(tt.script).ParseScript() + require.NoError(t, err) + require.Len(t, script.Cmds, 1) + require.Len(t, script.Cmds[0].Args, 1) + assert.Equal(t, tt.want, script.Cmds[0].Args[0]) + }) + } }