Skip to content

Conversation

@Scheremo
Copy link
Contributor

@Scheremo Scheremo commented Oct 6, 2025

Profiling showed visitDefault dominated indexing time (~99.99% of runtime in larger projects) due to deep traversal into non-relevant buffers. This patch keeps correctness while greatly reducing unnecessary recursion. In a SoC-scale System verilog project, this change reduces indexing time from 6.6 s to 4 ms.

  • Added definitelyOutsideMainBuffer() helper to detect whether an AST node’s source range, syntax range, or location belongs to the main buffer.
  • Integrated this check into the templated visit<T> function:
    • Skips recursion (visitDefault) when a node is clearly outside the main file.
    • Uses a new ASTBase alias for clarity and correctness when calling visitDefault<T>().
  • Removed redundant visitDefault() calls from individual symbol visitors (NetSymbol, VariableSymbol, ImportSymbol, etc.) that no longer need to recurse manually.
  • Result: traversal remains correct but prunes large off-buffer branches early, improving indexing time for multi-file compilations.

@Scheremo Scheremo marked this pull request as ready for review October 6, 2025 08:54
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Cool! I think this totally makes sense.

Profiling showed `visitDefault` dominated indexing time (~99.99% of runtime in larger projects) due
to deep traversal into non-relevant buffers. This patch keeps correctness while
greatly reducing unnecessary recursion. In a SoC-scale System verilog project, this change reduces
indexing time from 6.6 s to 4 ms.

- Added `definitelyOutsideMainBuffer()` helper to detect whether an AST node’s
  source range, syntax range, or location belongs to the main buffer.
- Integrated this check into the templated `visit<T>` function:
  - Skips recursion (`visitDefault`) when a node is clearly outside the main file.
  - Uses a new `ASTBase` alias for clarity and correctness when calling
    `visitDefault<T>()`.
- Removed redundant `visitDefault()` calls from individual symbol visitors
  (`NetSymbol`, `VariableSymbol`, `ImportSymbol`, etc.) that no longer need to
  recurse manually.
- Result: traversal remains correct but prunes large off-buffer branches early,
  improving indexing time for multi-file compilations.
@Scheremo Scheremo merged commit e6674ee into llvm:main Oct 6, 2025
7 checks passed
@Scheremo Scheremo deleted the pr-lsp-indexing branch October 21, 2025 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants