From e04f141816a2606057d5aef5962970b7559b40e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9mence=20Lesn=C3=A9?= Date: Wed, 28 Oct 2026 10:52:02 +0100 Subject: [PATCH 1/3] keep ctags tags whose reply line is 4 KiB or longer The scanner read ctags replies through a 4,096-byte bufio buffer and silently skipped every line that did not fit (from zoekt 5851b63). Parse then lost the tag with no error and no log. Since `scope` repeats the parent's full name, the children of a long-named type were lost too. Lines that do not fit in the buffer are now copied into a reused buffer, up to maxLineSize (1 MiB). Short lines still go through ReadSlice with no copy. Lines over the cap are skipped as before, and Parse logs one Info line per file when it skipped anything. Long lines that are not tags, or that do not decode, are still skipped instead of becoming errors, so Parse and New fail on nothing they accepted before. Unmarshal errors quote at most 200 bytes of the line. Scan clears `long` when it returns false, so that Parse returns an error, as before, when ctags dies in the middle of a long line. The new tests assert names, lines and parents rather than goldens, so they hold on ctags 6.0.0 and 6.2.1. fake_test.go lets the test binary stand in for ctags to script exact replies; writeFakeScript covers the cases it cannot (startup output, a reply cut short). Memory is bounded in the next commit. Refs #27 Co-Authored-By: Claude Opus 5.5 --- ctags.go | 126 ++++++++++++------ ctags_test.go | 352 +++++++++++++++++++++++++++++++++++++++++++++++++- fake_test.go | 53 ++++++++ 3 files changed, 489 insertions(+), 42 deletions(-) create mode 100644 fake_test.go diff --git a/ctags.go b/ctags.go index d928659..18fc5fe 100644 --- a/ctags.go +++ b/ctags.go @@ -85,7 +85,7 @@ func New(opts Options) (Parser, error) { proc := ctagsProcess{ cmd: cmd, in: in, - out: &scanner{r: bufio.NewReaderSize(out, 4096)}, + out: &scanner{r: bufio.NewReaderSize(out, 4096), maxLen: maxLineSize}, outPipe: out, Info: opts.Info, @@ -141,27 +141,38 @@ func (p *ctagsProcess) Close() { } func (p *ctagsProcess) read(rep *reply) error { - if !p.out.Scan() { - // Some errors do not kill the parser. We would deadlock if we waited - // for the process to exit. - err := p.out.Err() - p.Close() - return err - } - if p.Debug != nil { - p.Debug.Printf("read %q", p.out.Bytes()) - } + for { + if !p.out.Scan() { + // Some errors do not kill the parser. We would deadlock if we waited + // for the process to exit. + err := p.out.Err() + p.Close() + return err + } + if p.Debug != nil { + p.Debug.Printf("read %q", p.out.Bytes()) + } - // See https://github.com/universal-ctags/ctags/issues/1493 - if bytes.Equal([]byte("(null)"), p.out.Bytes()) { - return nil - } + // See https://github.com/universal-ctags/ctags/issues/1493 + if bytes.Equal([]byte("(null)"), p.out.Bytes()) { + return nil + } - err := json.Unmarshal(p.out.Bytes(), rep) - if err != nil { - return fmt.Errorf("unmarshal(%q): %v", p.out.Bytes(), err) + err := json.Unmarshal(p.out.Bytes(), rep) + if p.out.long && (err != nil || rep.Typ != "tag") { + // Skip long lines that are not tags: before issue #27 every long + // line was skipped, so callers never got an error for one. Clear + // rep, which Unmarshal may have filled in part. + *rep = reply{} + p.out.skipped++ + continue + } + if err != nil { + // Quote at most 200 bytes: a line can be up to maxLineSize. + return fmt.Errorf("unmarshal(%.200q): %v", p.out.Bytes(), err) + } + return nil } - return nil } // universal-ctags line buffer size is only 1024. @@ -250,6 +261,8 @@ func (p *ctagsProcess) Parse(name string, content []byte) ([]*Entry, error) { return nil, nil } + p.out.skipped = 0 + // 250 is a better guess for initial size es := make([]*Entry, 0, 250) for { @@ -259,6 +272,9 @@ func (p *ctagsProcess) Parse(name string, content []byte) ([]*Entry, error) { } switch rep.Typ { case "completed": + if n := p.out.skipped; n > 0 && p.Info != nil { + p.Info.Printf("ctags skipping %d long output lines for %s", n, name) + } return es, nil case "error": if rep.Fatal { @@ -287,39 +303,67 @@ func (p *ctagsProcess) Parse(name string, content []byte) ([]*Entry, error) { } } -// scanner is like bufio.Scanner but skips long lines instead of returning -// bufio.ErrTooLong. +const ( + // maxLineSize is the longest ctags output line we read. Longer lines, + // and so the tags they describe, are skipped. + maxLineSize = 1 << 20 +) + +// scanner is like bufio.Scanner but skips lines longer than maxLen instead of +// returning bufio.ErrTooLong. Lines that do not fit in r's buffer are copied +// into a reused buffer, and Scan sets long for them. // // Additionally it will skip empty lines. type scanner struct { - r *bufio.Reader - line []byte - err error + r *bufio.Reader + maxLen int // excluding the trailing newline, including any \r before it + buf []byte + line []byte + long bool // line did not fit in r's buffer + err error + + skipped int // long lines skipped: longer than maxLen, or not tags (see read) } func (s *scanner) Scan() bool { - if s.err != nil { - return false - } - - var ( - err error - line []byte - ) - - for err == nil && len(line) == 0 { - line, err = s.r.ReadSlice('\n') - for err == bufio.ErrBufferFull { - // make line empty so we ignore it - line = nil - _, err = s.r.ReadSlice('\n') + for s.err == nil { + line, err := s.r.ReadSlice('\n') + s.long = err == bufio.ErrBufferFull + if s.long { + line, err = s.readLong(line) } + s.err = err + line = bytes.TrimSuffix(line, []byte{'\n'}) + if len(line) > s.maxLen { + s.skipped++ + continue + } line = bytes.TrimSuffix(line, []byte{'\r'}) + if len(line) > 0 { + s.line = line + return true + } } + s.line, s.long = nil, false + return false +} - s.line, s.err = line, err - return len(line) > 0 +// readLong reads the rest of a line that did not fit in r's buffer. It stops +// copying once the line is longer than maxLen (the result is then cut, and +// Scan skips it), but still consumes the whole line so that the next Scan +// starts on a line boundary. +func (s *scanner) readLong(line []byte) ([]byte, error) { + s.buf = append(s.buf[:0], line...) + for { + line, err := s.r.ReadSlice('\n') + if len(s.buf) <= s.maxLen+1 { + s.buf = append(s.buf, line...) + } + if err != bufio.ErrBufferFull { + return s.buf, err + } + } } func (s *scanner) Bytes() []byte { diff --git a/ctags_test.go b/ctags_test.go index e13dbc9..fe41301 100644 --- a/ctags_test.go +++ b/ctags_test.go @@ -2,12 +2,16 @@ package ctags import ( "bufio" + "bytes" "context" + "fmt" + "io" "log" "os" "path/filepath" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/hexops/autogold" @@ -120,6 +124,7 @@ def g = {x -> x} func TestScanner(t *testing.T) { size := 20 + maxLen := 2*size + 1 input := strings.Join([]string{ "aaaaaaaaa", @@ -128,17 +133,21 @@ func TestScanner(t *testing.T) { "", strings.Repeat("e", size-1), "f\r", + strings.Repeat("G", maxLen+1), "gg", }, "\n") + // Lines longer than the buffer are kept. Lines longer than maxLen are + // skipped: the B's, and the G's which are one byte over. want := []string{ "aaaaaaaaa", + strings.Repeat("C", size) + strings.Repeat("D", size+1), strings.Repeat("e", size-1), "f", "gg", } var got []string - r := &scanner{r: bufio.NewReaderSize(strings.NewReader(input), size)} + r := newScanner(strings.NewReader(input), size, maxLen) for r.Scan() { got = append(got, string(r.Bytes())) } @@ -151,6 +160,88 @@ func TestScanner(t *testing.T) { } } +// TestScannerBufferBoundary pins the 4,096-byte boundary of issue #27: with +// the 4,096-byte buffer that New uses, a 4,095-byte line fits and a 4,096-byte +// line does not. Both must be returned, and only the second is reported as +// long: the lines that were skipped before the fix. +func TestScannerBufferBoundary(t *testing.T) { + const size = 4096 + lines := []string{ + strings.Repeat("a", size-1), + strings.Repeat("b", size), + strings.Repeat("c", 3*size), + "d", + } + wantLong := []bool{false, true, true, false} + + r := newScanner(strings.NewReader(strings.Join(lines, "\n")+"\n"), size, 1<<20) + var got []string + var gotLong []bool + for r.Scan() { + got = append(got, string(r.Bytes())) + gotLong = append(gotLong, r.long) + } + if err := r.Err(); err != nil { + t.Fatal(err) + } + if len(got) != len(lines) { + t.Fatalf("got %d lines, want %d", len(got), len(lines)) + } + for i := range lines { + if got[i] != lines[i] { + t.Errorf("line %d: got %d bytes, want %d", i, len(got[i]), len(lines[i])) + } + } + if !cmp.Equal(gotLong, wantLong) { + t.Errorf("long: got %v, want %v", gotLong, wantLong) + } +} + +// FuzzScanner checks what callers rely on: lines come back in order and +// intact (without "\r\n"), every non-empty line shorter than maxLen comes +// back, and no line longer than maxLen does. +func FuzzScanner(f *testing.F) { + f.Add([]byte("a\nbb\r\n\n\r\nccc"), uint8(0), uint16(2)) + f.Add([]byte(strings.Repeat("x", 40)+"\n"+strings.Repeat("y", 15)+"\r\nz"), uint8(0), uint16(39)) + f.Add([]byte(strings.Repeat("x", 16)), uint8(0), uint16(100)) + f.Add([]byte(strings.Repeat("x", 20)+"\nok"), uint8(0), uint16(16)) // first chunk is exactly maxLen + f.Fuzz(func(t *testing.T, data []byte, sz uint8, ml uint16) { + size := 16 + int(sz%48) // bufio's minimum is 16 + maxLen := int(ml % 256) + + var got []string + r := newScanner(bytes.NewReader(data), size, maxLen) + for r.Scan() { + got = append(got, string(r.Bytes())) + } + if err := r.Err(); err != nil { + t.Fatal(err) + } + if r.Scan() { + t.Fatal("Scan returned true after returning false") + } + + // Walk the input lines and the returned lines together. + rest := got + for _, line := range bytes.Split(data, []byte{'\n'}) { + line = bytes.TrimSuffix(line, []byte{'\r'}) + if len(rest) > 0 && rest[0] == string(line) { + if len(line) > maxLen { + t.Fatalf("size=%d maxLen=%d: returned a %d-byte line", size, maxLen, len(line)) + } + rest = rest[1:] + continue + } + if len(line) > 0 && len(line) < maxLen { + t.Fatalf("size=%d maxLen=%d: dropped %q", size, maxLen, line) + } + } + if len(rest) > 0 { + t.Fatalf("size=%d maxLen=%d: returned %q, not an input line in order", size, maxLen, rest[0]) + } + }) +} + func TestLanguageMapping(t *testing.T) { mapping, err := ListLanguageMappings(context.Background(), os.Getenv("CTAGS_COMMAND")) if err != nil { @@ -167,3 +258,262 @@ func TestLanguageMapping(t *testing.T) { t.Fatalf("unexpected mappings list for 'JavaScript': got=%v expected=%v", list, expectedList) } } + +// newScanner returns a scanner over r with a read buffer of size bytes. +func newScanner(r io.Reader, size, maxLen int) *scanner { + return &scanner{r: bufio.NewReaderSize(r, size), maxLen: maxLen} +} + +// Tests for https://github.com/sourcegraph/go-ctags/issues/27: tags whose +// reply line has 4,096 bytes or more. They assert names, lines and parents +// (not goldens) so that they hold on ctags 6.0.0 and 6.2.1 alike. + +func newTestParser(t *testing.T, info *bytes.Buffer) Parser { + t.Helper() + opts := Options{Bin: os.Getenv("CTAGS_COMMAND")} + if info != nil { + opts.Info = log.New(info, "", 0) + } + p, err := New(opts) + if err != nil { + t.Fatal(err) + } + t.Cleanup(p.Close) + if info != nil { + // ctags 6.2 logs initialization warnings (TOML, Cargo) that 6.0 + // does not. Only check what Parse logs. + info.Reset() + } + return p +} + +// tag is the part of an Entry the tests check. Long strings are shortened +// to a prefix and a length so that failures stay readable. +type tag struct { + Name string + Line int + Parent string + ParentKind string +} + +func short(s string) string { + if len(s) <= 16 { + return s + } + return fmt.Sprintf("%s...(%d)", s[:8], len(s)) +} + +func tags(es []*Entry) []tag { + var ts []tag + for _, e := range es { + ts = append(ts, tag{short(e.Name), e.Line, short(e.Parent), e.ParentKind}) + } + return ts +} + +func TestParseLongLines(t *testing.T) { + p := newTestParser(t, nil) + + bigClass := "Big" + strings.Repeat("x", 5000) + bigStruct := "T" + strings.Repeat("y", 5000) + issueFunc := "Long" + strings.Repeat("x", 5000) // the issue's example: 5,004 B + func32K := "F" + strings.Repeat("x", 32<<10) // inside the 4–64 KiB range that worked before Zoekt 5851b63 + func100K := "F" + strings.Repeat("y", 100<<10) // beyond 64 KiB, under the cap + var params []string + for i := 0; i < 500; i++ { + params = append(params, fmt.Sprintf("int p%d", i)) + } + + cases := []struct { + name, path, src string + want []tag + }{{ + // S1: children repeat the parent's full name in scope. + name: "python class subtree", + path: "a.py", + src: "class " + bigClass + ":\n def a(self): pass\n def b(self): pass\n\ndef tail(): pass\n", + want: []tag{ + {short(bigClass), 1, "", ""}, + {"a", 2, short(bigClass), "class"}, + {"b", 3, short(bigClass), "class"}, + {"tail", 5, "", ""}, + }, + }, { + name: "go struct subtree", + path: "t.go", + src: "package p\n\ntype " + bigStruct + " struct {\n\tA int\n\tB string\n}\n", + want: []tag{ + {"p", 1, "", ""}, + {short(bigStruct), 3, "p", "package"}, + {"A", 4, short("p." + bigStruct), "struct"}, + {"B", 5, short("p." + bigStruct), "struct"}, + }, + }, { + // S2: ctags does not cut the name. + name: "go func name from issue", + path: "sample.go", + src: "package sample\n\nfunc Short() {}\n\nfunc " + issueFunc + "() {}\n\nfunc After() {}\n", + want: []tag{ + {"sample", 1, "", ""}, + {"Short", 3, "sample", "package"}, + {short(issueFunc), 5, "sample", "package"}, + {"After", 7, "sample", "package"}, + }, + }, { + // S2: ctags does not cut the signature (checked below). + name: "c signature from issue", + path: "c.c", + src: "int f(" + strings.Join(params, ", ") + ") { return 0; }\n", + want: []tag{{"f", 1, "", ""}}, + }, { + // S3 + name: "32 KiB name", + path: "m.go", + src: "package m\n\nfunc " + func32K + "() {}\n", + want: []tag{{"m", 1, "", ""}, {short(func32K), 3, "m", "package"}}, + }, { + name: "100 KiB name", + path: "h.go", + src: "package h\n\nfunc " + func100K + "() {}\n", + want: []tag{{"h", 1, "", ""}, {short(func100K), 3, "h", "package"}}, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + es, err := p.Parse(tc.path, []byte(tc.src)) + if err != nil { + t.Fatal(err) + } + if d := cmp.Diff(tc.want, tags(es)); d != "" { + t.Fatalf("tags (-want +got):\n%s", d) + } + for _, e := range es { + for _, s := range []string{bigClass, "p." + bigStruct, issueFunc, func32K, func100K} { + if short(e.Name) == short(s) && e.Name != s || short(e.Parent) == short(s) && e.Parent != s { + t.Errorf("%s: long string changed", e.Name) + } + } + } + if tc.path == "c.c" { + if want := "(" + strings.Join(params, ",") + ")"; es[0].Signature != want { + t.Errorf("signature: got %d bytes, want %d", len(es[0].Signature), len(want)) + } + } + }) + } +} + +// Lines over the 1 MiB cap are skipped with no error and one Info line, and +// the stream stays in sync. Lines between 4 KiB and the cap are kept. +func TestParseLineLengthLimit(t *testing.T) { + var info bytes.Buffer + p := newTestParser(t, &info) + + kept := "K" + strings.Repeat("x", 512<<10) // over the 4 KiB buffer, under the cap + skipped := "S" + strings.Repeat("y", 2<<20) // over the cap + src := "package p\n\nfunc Short() {}\n\nfunc " + kept + "() {}\n\nfunc " + skipped + "() {}\n\nfunc After() {}\n" + es, err := p.Parse("a.go", []byte(src)) + if err != nil { + t.Fatal(err) + } + want := []tag{ + {"p", 1, "", ""}, + {"Short", 3, "p", "package"}, + {short(kept), 5, "p", "package"}, + {"After", 9, "p", "package"}, + } + if d := cmp.Diff(want, tags(es)); d != "" { + t.Errorf("tags (-want +got):\n%s", d) + } + if n := strings.Count(info.String(), "\n"); n != 1 || !strings.Contains(info.String(), "skipping 1 long output lines") { + t.Errorf("want one Info line about 1 skipped line, got %q", info.String()) + } + + info.Reset() + es, err = p.Parse("b.go", []byte("package q\nfunc Next() {}\n")) + if err != nil || len(es) != 2 || info.Len() != 0 { + t.Errorf("next Parse: tags %v, err %v, info %q", tags(es), err, info.String()) + } +} + +// Long lines that are not valid tags were skipped unread before the fix for +// issue #27. They must still be skipped, not fail Parse. +func TestParseLongNonTagLines(t *testing.T) { + var info bytes.Buffer + p := newFakeParser(t, Options{Info: log.New(&info, "", 0)}) + long := strings.Repeat("x", 5000) + reply := `{"_type": "tag", "name": "Before", "path": "a.go", "line": 1, "kind": "func"} +{"_type": "error", "message": "` + long + `", "fatal": true} +{"_type": "ptag", "name": "` + long + `"} +not json ` + long + ` +{"_type": "tag", "scope": "Stale", "scopeKind": "class", "name": "` + long + `", "line": "1"} +{"_type": "tag", "name": "After", "path": "a.go", "line": 2, "kind": "func"} +{"_type": "completed", "command": "generate-tags"} +` + for _, name := range []string{"a.go", "b.go"} { + es, err := p.Parse(name, []byte(reply)) + if err != nil { + t.Fatal(err) + } + want := []tag{{"Before", 1, "", ""}, {"After", 2, "", ""}} + if d := cmp.Diff(want, tags(es)); d != "" { + t.Fatalf("%s: tags (-want +got):\n%s", name, d) + } + if want := "ctags skipping 4 long output lines for " + name + "\n"; info.String() != want { + t.Errorf("info: got %q, want %q", info.String(), want) + } + info.Reset() + } +} + +// When the output ends in the middle of a long line, for example because ctags +// was killed, Parse must return an error as it did before, not loop forever. +func TestParseEOFInLongLine(t *testing.T) { + // Answer the first request with half of a long line, then exit. + bin := writeFakeScript(t, `echo '{"_type": "program", "name": "Universal Ctags"}'`+"\n"+ + "read -r request\n"+ + "printf '%s' '"+strings.Repeat("x", 5000)+"'\n") + p, err := New(Options{Bin: bin}) + if err != nil { + t.Fatal(err) + } + defer p.Close() + + done := make(chan error, 1) + go func() { + _, err := p.Parse("a.go", nil) + done <- err + }() + select { + case err := <-done: + if err == nil { + t.Fatal("expected an error") + } + case <-time.After(5 * time.Second): + t.Fatal("Parse did not return after ctags exited") + } +} + +// Long lines printed while ctags starts were skipped unread before the fix +// for issue #27. They must not fail New. +func TestNewLongInitLine(t *testing.T) { + bin := writeFakeScript(t, `echo '{"_type": "error", "message": "`+strings.Repeat("m", 5000)+`", "fatal": true}'`+"\n"+ + `echo '{"_type": "program", "name": "Universal Ctags"}'`+"\n"+ + "cat >/dev/null\n") + p, err := New(Options{Bin: bin}) + if err != nil { + t.Fatal(err) + } + p.Close() +} + +// writeFakeScript writes a shell script that stands in for ctags, for replies +// that fakeCtags cannot script, such as output during startup. +func writeFakeScript(t *testing.T, body string) string { + t.Helper() + bin := filepath.Join(t.TempDir(), "ctags") + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"+body), 0o755); err != nil { + t.Fatal(err) + } + return bin +} diff --git a/fake_test.go b/fake_test.go new file mode 100644 index 0000000..32f1122 --- /dev/null +++ b/fake_test.go @@ -0,0 +1,53 @@ +package ctags + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "os" + "testing" +) + +// TestMain lets the test binary stand in for universal-ctags when +// GO_CTAGS_FAKE is set. See fakeCtags. +func TestMain(m *testing.M) { + if os.Getenv("GO_CTAGS_FAKE") != "" { + fakeCtags() + return + } + os.Exit(m.Run()) +} + +// fakeCtags speaks the interactive protocol, but replies to each +// generate-tags request with the request's content as is. This lets tests +// script the exact reply lines ctags would print. +func fakeCtags() { + fmt.Println(`{"_type": "program", "name": "Universal Ctags"}`) + r := bufio.NewReader(os.Stdin) + for { + line, err := r.ReadBytes('\n') + if err != nil { + return + } + var req request + if err := json.Unmarshal(line, &req); err != nil { + return + } + if _, err := io.CopyN(os.Stdout, r, int64(req.Size)); err != nil { + return + } + } +} + +// newFakeParser returns a Parser backed by fakeCtags. +func newFakeParser(t *testing.T, opts Options) Parser { + t.Setenv("GO_CTAGS_FAKE", "1") + opts.Bin = os.Args[0] + p, err := New(opts) + if err != nil { + t.Fatal(err) + } + t.Cleanup(p.Close) + return p +} From 394a0017775e4c8468d082845174402f8f97f64a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9mence=20Lesn=C3=A9?= Date: Wed, 28 Oct 2026 10:52:31 +0100 Subject: [PATCH 2/3] bound the memory kept from long ctags reply lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading long lines (previous commit) removes the only bound go-ctags had on the size of its output. Every child tag repeats its parent's full name in `scope`, so a small file can produce a huge reply: a 1 MB Python file with one class whose name is 1 MB long and 200 inner classes makes ctags print 401 MB. Zoekt keeps those strings until the shard is written. Two measures, both per Parse: - Parent names of minSharedParentLen (256) bytes or more are shared through a map, so N children of one long parent keep one copy of its name. Shorter names are not worth a map entry. On a 232 KB Go file with a 3.5 KB struct name and 20,000 fields (every line under 4 KiB) this cuts the retained heap from 85 MB to 3.5 MB. - Lines that do not fit in the read buffer count against a budget of max(1 MiB, 100 × len(content)) reply bytes, a shared parent counted once. Once a tag does not fit, Parse lowers the scanner's limit so that every later long line of that call is skipped without being decoded. Lines under 4 KiB never count, so Parse still returns every tag it did before the previous commit. The limits are unexported constants; ratio and minBudget are fields only so that tests can lower them. On ordinary files the Go side of Parse shows no measurable difference in time or allocations. Fixes #27 Co-Authored-By: Claude Opus 5.5 --- ctags.go | 57 +++++++++++++++++++++++- ctags_test.go | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 2 deletions(-) diff --git a/ctags.go b/ctags.go index 18fc5fe..a5feb08 100644 --- a/ctags.go +++ b/ctags.go @@ -88,6 +88,9 @@ func New(opts Options) (Parser, error) { out: &scanner{r: bufio.NewReaderSize(out, 4096), maxLen: maxLineSize}, outPipe: out, + ratio: outputRatioLimit, + minBudget: maxLineSize, + Info: opts.Info, Debug: opts.Debug, } @@ -129,6 +132,12 @@ type ctagsProcess struct { out *scanner outPipe io.ReadCloser + // ratio and minBudget set the budget of each Parse: at most + // max(minBudget, ratio*len(content)) bytes kept from lines that do not + // fit in the read buffer. New sets them to outputRatioLimit and + // maxLineSize. They are fields so that tests can lower them. + ratio, minBudget int + Info *log.Logger Debug *log.Logger } @@ -261,7 +270,16 @@ func (p *ctagsProcess) Parse(name string, content []byte) ([]*Entry, error) { return nil, nil } - p.out.skipped = 0 + // Every child tag repeats the full name of its parent in scope, so tags + // can be much larger than the file. Share parent strings between tags, + // and bound the bytes kept from lines that do not fit in the read buffer. + // Only those lines count against the budget: shorter lines were always + // kept, so Parse still returns every tag it did before. Once a tag does + // not fit in the budget, every later long line in this Parse is skipped. + budget := max(p.minBudget, p.ratio*len(content)) + used, dropped := 0, 0 + parents := map[string]string{} + p.out.maxLen, p.out.skipped = maxLineSize, 0 // 250 is a better guess for initial size es := make([]*Entry, 0, 250) @@ -272,7 +290,7 @@ func (p *ctagsProcess) Parse(name string, content []byte) ([]*Entry, error) { } switch rep.Typ { case "completed": - if n := p.out.skipped; n > 0 && p.Info != nil { + if n := p.out.skipped + dropped; n > 0 && p.Info != nil { p.Info.Printf("ctags skipping %d long output lines for %s", n, name) } return es, nil @@ -283,6 +301,32 @@ func (p *ctagsProcess) Parse(name string, content []byte) ([]*Entry, error) { p.Info.Printf("ignoring non-fatal ctags error for %s: %s", name, rep.Message) } case "tag": + // Short scopes are not worth a map entry: sharing them saves + // little and costs more allocations on every ordinary file. + intern := len(rep.Scope) >= minSharedParentLen + var parent string + shared := false + if intern { + parent, shared = parents[rep.Scope] + } + if p.out.long { + // A shared parent costs nothing more: it is kept once. + cost := len(p.out.Bytes()) - len(parent) + if cost > budget-used { + // The budget is spent. Lower the scanner's limit below + // the buffer size so that it skips every later long line + // without decoding it. Parse resets it on the next call. + p.out.maxLen = p.out.r.Size() - 1 + dropped++ + continue + } + used += cost + } + if shared { + rep.Scope = parent + } else if intern { + parents[rep.Scope] = rep.Scope + } if rep.Path == filename { rep.Path = name } @@ -307,6 +351,15 @@ const ( // maxLineSize is the longest ctags output line we read. Longer lines, // and so the tags they describe, are skipped. maxLineSize = 1 << 20 + + // minSharedParentLen is the shortest parent name that Parse shares + // between the tags of a file. + minSharedParentLen = 256 + + // outputRatioLimit bounds the bytes kept per Parse from lines that do not + // fit in the read buffer to this many times the size of the content (but + // at least maxLineSize). + outputRatioLimit = 100 ) // scanner is like bufio.Scanner but skips lines longer than maxLen instead of diff --git a/ctags_test.go b/ctags_test.go index fe41301..7a0883e 100644 --- a/ctags_test.go +++ b/ctags_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" "time" + "unsafe" "github.com/google/go-cmp/cmp" "github.com/hexops/autogold" @@ -264,6 +265,11 @@ func newScanner(r io.Reader, size, maxLen int) *scanner { return &scanner{r: bufio.NewReaderSize(r, size), maxLen: maxLen} } +// setOutputBudget lowers the budget of p: max(minBudget, ratio*len(content)). +func setOutputBudget(p Parser, ratio, minBudget int) { + p.(*ctagsProcess).ratio, p.(*ctagsProcess).minBudget = ratio, minBudget +} + // Tests for https://github.com/sourcegraph/go-ctags/issues/27: tags whose // reply line has 4,096 bytes or more. They assert names, lines and parents // (not goldens) so that they hold on ctags 6.0.0 and 6.2.1 alike. @@ -436,6 +442,84 @@ func TestParseLineLengthLimit(t *testing.T) { } } +// N children of one parent keep one copy of its name, whether their lines fit +// in the read buffer or not. Long lines charge the shared name only once. +func TestParseSharedParent(t *testing.T) { + for _, n := range []int{3000, 20000} { + t.Run(fmt.Sprintf("%d B name", n), func(t *testing.T) { + p := newTestParser(t, nil) + // Charging the 20 KB parent to every field would cost 200 × 20 KB + // = 4 MB. Shared, the file costs about 70 KB. + setOutputBudget(p, 1, 256<<10) + + var b strings.Builder + b.WriteString("package p\n\ntype T" + strings.Repeat("y", n) + " struct {\n") + for i := 0; i < 200; i++ { + fmt.Fprintf(&b, "\tF%d int\n", i) + } + b.WriteString("}\n") + es, err := p.Parse("shared.go", []byte(b.String())) + if err != nil { + t.Fatal(err) + } + if len(es) != 202 { + t.Fatalf("got %d tags, want 202 (package, struct, 200 fields)", len(es)) + } + for _, e := range es[3:] { + if unsafe.StringData(e.Parent) != unsafe.StringData(es[2].Parent) { + t.Fatalf("%s: parent string is not shared", e.Name) + } + } + }) + } +} + +// Distinct long scopes cannot be shared: tags from long lines past the budget +// are dropped with no error and one Info line. Tags from short lines (which +// main returns) are always kept. +func TestParseOutputBudget(t *testing.T) { + var info bytes.Buffer + p := newTestParser(t, &info) + setOutputBudget(p, 2, 0) // budget = 2 × the file size, about 45 KB + + // Each x has the distinct 20 KB scope "A<20 KB>.Bi". + var b strings.Builder + b.WriteString("class A" + strings.Repeat("y", 20000) + ":\n") + for i := 0; i < 50; i++ { + fmt.Fprintf(&b, " class B%d:\n x = 1\n", i) + } + b.WriteString("\nclass Small:\n y = 1\n\ndef tail(): pass\n") + + es, err := p.Parse("budget.py", []byte(b.String())) + if err != nil { + t.Fatal(err) + } + got := map[string]bool{} + for _, e := range es { + got[e.Name] = true + } + if !got["A"+strings.Repeat("y", 20000)] { + t.Errorf("first long tag was dropped: %v", tags(es)) + } + for _, n := range []string{"Small", "y", "tail"} { + if !got[n] { + t.Errorf("short tag %s was dropped", n) + } + } + if len(es) >= 104 { + t.Errorf("got all %d tags, want some dropped", len(es)) + } + if n := strings.Count(info.String(), "\n"); n != 1 || !strings.Contains(info.String(), "long output lines") { + t.Errorf("want one Info line about the budget, got %q", info.String()) + } + t.Logf("kept %d of 104 tags; %s", len(es), info.String()) + + es, err = p.Parse("b.go", []byte("package q\nfunc Next() {}\n")) + if err != nil || len(es) != 2 { + t.Errorf("next Parse: tags %v, err %v", tags(es), err) + } +} + // Long lines that are not valid tags were skipped unread before the fix for // issue #27. They must still be skipped, not fail Parse. func TestParseLongNonTagLines(t *testing.T) { @@ -466,6 +550,40 @@ not json ` + long + ` } } +// Once a long tag does not fit in the budget, every later long line of that +// Parse is skipped, even one that would still fit. Short lines are kept, and +// the next Parse starts with a fresh budget. +func TestParseOutputBudgetStops(t *testing.T) { + var info bytes.Buffer + p := newFakeParser(t, Options{Info: log.New(&info, "", 0)}) + setOutputBudget(p, 0, 10000) // room for one 5 KB line, not two + + longTag := func(i int) string { + return fmt.Sprintf(`{"_type": "tag", "name": "x%d", "line": %d, "scope": "S%d%s"}`+"\n", i, i+1, i, strings.Repeat("y", 5000)) + } + // A tag line of exactly 4,096 bytes, the shortest long line. It would + // fit in what is left of the budget after x0. + edge := `{"_type": "tag", "line": 9, "name": "e` + edge += strings.Repeat("e", 4096-len(edge)-2) + `"}` + "\n" + completed := `{"_type": "completed", "command": "generate-tags"}` + "\n" + + es, err := p.Parse("a.go", []byte(longTag(0)+longTag(1)+edge+longTag(2)+`{"_type": "tag", "name": "short", "line": 4}`+"\n"+completed)) + if err != nil { + t.Fatal(err) + } + if d := cmp.Diff([]tag{{"x0", 1, short("S0" + strings.Repeat("y", 5000)), ""}, {"short", 4, "", ""}}, tags(es)); d != "" { + t.Fatalf("tags (-want +got):\n%s", d) + } + if want := "ctags skipping 3 long output lines for a.go\n"; info.String() != want { + t.Errorf("info: got %q, want %q", info.String(), want) + } + + es, err = p.Parse("b.go", []byte(longTag(3)+completed)) + if err != nil || len(es) != 1 { + t.Errorf("next Parse: tags %v, err %v", tags(es), err) + } +} + // When the output ends in the middle of a long line, for example because ctags // was killed, Parse must return an error as it did before, not loop forever. func TestParseEOFInLongLine(t *testing.T) { From cf6914cbd4cbf1f862d387200a39cc49704fdaa5 Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 28 Oct 2026 12:18:05 +0000 Subject: [PATCH 3/3] lower the long-line budget to 10x the content size Zoekt keeps every file's tags in memory until the shard is written, so 100x let a single pathological 1 MB file hold on to 100 MB. The budget only counts lines that do not fit in the read buffer, so lowering it never drops a tag that was kept before long lines were read. Amp-Thread-ID: https://ampcode.com/threads/T-01a18240-4e65-7468-b990-c9c16c76124f Co-authored-by: Keegan Smith --- ctags.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ctags.go b/ctags.go index a5651ad..68c557d 100644 --- a/ctags.go +++ b/ctags.go @@ -378,8 +378,9 @@ const ( // outputRatioLimit bounds the bytes kept per Parse from lines that do not // fit in the read buffer to this many times the size of the content (but - // at least maxLineSize). - outputRatioLimit = 100 + // at least maxLineSize). Zoekt holds every file's tags in memory until + // the shard is written, so this stays low. + outputRatioLimit = 10 ) // scanner is like bufio.Scanner but skips lines longer than maxLen instead of