Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

Conversation

@efritz
Copy link
Contributor

@efritz efritz commented Jul 28, 2020

The current implementation of lsif-go contains a super-quadratic way of determining comments based on token positions. For each definition, we trace the AST from the root of the containing file through nodes that intersect the target position (via astutil.PathEnclosingInterval) then extract the comments by tracing the constructed ancestor path backwards.

This requires us to scan the same file from the root for every definition that exists in that file. Much of this work will be repeated and we'll end up tracing the same path multiple times to get to definitions with slightly different paths. Tell me that's not a bad idea in a world containing files of this size.

This change introduces a pre-warmed cache of hover text that is extracted by traversing the AST of each file once. We further speed up this process by walking GOMAXPROC files in parallel.

  • Before this change, aws-sdk-go indexes in 7m11s.
  • After this change, aws-sdk-go indexes in 2m9s.

@efritz efritz changed the title Parallel load Single-pass hover text extraction Jul 28, 2020
@efritz efritz requested review from garobrik and unknwon July 28, 2020 20:43
@efritz efritz marked this pull request as ready for review July 28, 2020 20:48
@keegancsmith
Copy link
Member

Drive by comment: I'm unsure how applicable this package is, but it might be a nice performant architecture for writing things which extract information from go. https://godoc.org/golang.org/x/tools/go/analysis

@efritz
Copy link
Contributor Author

efritz commented Jul 29, 2020

@keegancsmith this would be useful to report diagnostics but wouldn't replace any work here.

@keegancsmith
Copy link
Member

@keegancsmith this would be useful to report diagnostics but wouldn't replace any work here.

Should probably fork this into another discussion, but take a closer look at that package. It is a generic way to build tools which extract information from go programs in efficient ways. IE takes care of things like caching intermediate artefacts based on the build graph. However, given we don't really have a good way to do incremental work for lsif-go relying on packages makes a lot of sense.

@efritz efritz requested a review from aidaeology July 31, 2020 14:29
Copy link
Contributor

@aidaeology aidaeology left a comment

Choose a reason for hiding this comment

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

The index function is over 100 lines of code and invokes multiple loops and processes and there are no tests to determine the logic in that function is working as expected all cases. Please add tests for the core index function so we can validate that these logic changes aren't breaking.

@efritz efritz merged commit c165bff into master Aug 4, 2020
@efritz efritz deleted the parallel-load branch August 4, 2020 13:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants