Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indexing: clean up ctags parser wrapper #708

Merged
merged 9 commits into from
Dec 6, 2023
33 changes: 31 additions & 2 deletions ctags/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,39 @@
package ctags

import (
"fmt"
"log"
"os"
"time"

goctags "github.com/sourcegraph/go-ctags"
)

type Entry = goctags.Entry

// CTagsParser wraps go-ctags and delegates to the right process (like universal-ctags or scip-ctags).
// It is only safe for single-threaded use.
// It is only safe for single-threaded use. This wrapper also enforces a timeout on parsing a single
// document, which is important since documents can occasionally hang universal-ctags.
// documents which hang universal-ctags.
type CTagsParser struct {
bins ParserBinMap
parsers map[CTagsParserType]goctags.Parser
}

// parseTimeout is how long we wait for a response for parsing a single file
// in ctags. 1 minute is a very conservative timeout which we should only hit
// if ctags hangs.
const parseTimeout = time.Minute

func NewCTagsParser(bins ParserBinMap) CTagsParser {
return CTagsParser{bins: bins, parsers: make(map[CTagsParserType]goctags.Parser)}
}

type parseResult struct {
entries []*Entry
err error
}

func (lp *CTagsParser) Parse(name string, content []byte, typ CTagsParserType) ([]*Entry, error) {
if lp.parsers[typ] == nil {
parser, err := lp.newCTagsParser(typ)
Expand All @@ -43,8 +57,23 @@ func (lp *CTagsParser) Parse(name string, content []byte, typ CTagsParserType) (
lp.parsers[typ] = parser
}

deadline := time.NewTimer(parseTimeout)
defer deadline.Stop()

parser := lp.parsers[typ]
return parser.Parse(name, content)
recv := make(chan parseResult, 1)
go func() {
entry, err := parser.Parse(name, content)
recv <- parseResult{entries: entry, err: err}
}()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the overhead of a new goroutine is likely fine given we know ctags is already the bottleneck. I'd be interested to know if it pops up in profiles now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I didn't see any difference in job latency or in the CPU profiles


select {
case resp := <-recv:
return resp.entries, resp.err
case <-deadline.C:
// Error out since ctags hanging is a sign something bad is happening.
return nil, fmt.Errorf("ctags timedout after %s parsing %s", parseTimeout, name)
}
}

func (lp *CTagsParser) newCTagsParser(typ CTagsParserType) (goctags.Parser, error) {
Expand Down