Conversation
|
Docs preview: https://vide.pascal-lab.net/preview/pr-344/ |
Adds host_with_project (loads a real SystemVerilog directory into
AnalysisHost) and index_benchmarks_real_project, an ignored bench that
times cold load, cold parse, module index, semantic index, and the
semantic-index rebuild after touching one file.
Run with:
VIDE_BENCH_PROJECT=third_party/slang/tests/unittests/data \
cargo test -p ide --release --lib -- --ignored --nocapture index_benchmarks_real_project
ModuleIndex::for_source_root and other file iterators call item_tree on every file in a source root, including .map library-map files whose slang syntax root is LibraryMap rather than a compilation unit. The old assertion panicked on those files. Return an empty item tree for non-compilation-unit roots: library-map declarations are lowered via lower_library_map, not the item tree, so they contribute no items.
Adds index_benchmarks_module_index_profile, an ignored test that times
parse, macro file discovery, AST id map, owner table, and item-tree
residual per query, isolating the module-index bottleneck.
Run with:
VIDE_BENCH_PROJECT=third_party/slang/tests/unittests/data \
cargo test -p ide --release --lib -- --ignored --nocapture index_benchmarks_module_index_profile
included_source_end_order scanned events after each include and walked the include-parent chain per event, making record_source_order_scopes O(sources * events * depth). A self-including file hits slang's 1024 include-depth limit, producing 1025 sources/events and ~10^9 walks (~4.5s on a 3.6KB project). Trace events are a depth-first traversal of the include forest, so an included source's scope ends exactly when the stream returns to a shallower source. Replace the scan with a monotonic stack over events keyed by precomputed include depth: O(events). Empty includes default to include_order + 1 as before. Module index on slang's multi-file test data: 4.4s -> 48ms.
PathIdentityIndex tracked OS file identity ((dev,ino) on Unix, (volume,index) on Windows) to deduplicate hard links. Hard-linked source files are effectively nonexistent; symlinks are already covered by the canonical-path alias. This halves the per-insert syscalls (canonicalize + stat -> canonicalize only) and drops the winapi-util dependency.
path_file_ids rebuilt the full path-spelling index on every call and was invoked per file from source_preproc_file_ids, giving O(n^2) canonicalization. Make it a salsa tracked query keyed by a workspace singleton so it is computed once per revision.
lower_name only handled IdentifierName, IdentifierSelectName, and ScopedName, missing KeywordName (used for constructor 'new' keyword). This caused lower_subroutine_prototype to return None for class constructors, leaving body.subroutine unset and panicking later in db.subroutine(). Add KeywordName handling via as_keyword_name().keyword().
Two trace-building paths threw std::logic_error on data slang reports for real-world code, which escaped the FFI boundary and terminated the process: - Source macro argument/body tokens with missing token-origin metadata now map to an unavailable origin instead of throwing. - Overlapping macro usages at the same source range (a macro expanding to another macro at the same location) now emit the event without a call identity instead of throwing. Both are graceful degradations; downstream consumers already treat unavailable/missing origins as non-resolvable.
Adds file_semantic_index and file_module_edges timing to the module-index profile, with a per-file breakdown sorted by semantic-index cost to surface the worst files.
The per-file and per-root module/semantic/symbol index queries were plain functions, so every call rebuilt from scratch. Name resolution during index construction therefore rebuilt the whole-root module index once per module-related token (named port connections, parameters, instantiations), and a single-file change rebuilt the entire root. Track them as salsa queries so the module index is computed once per root and reused across tokens, and a one-file edit invalidates only that file's index plus the root merge. common_cells (214 files, 24k lines), B6 real-project benchmark: - semantic index cold: 304s -> 3.4s - semantic index rebuild: 310s -> 5.3s
set_parse_lru_capacity only sized parsed_profile and parse_src_for_compilation, leaving parsed_compilation_unit, source_preproc_model, macro expansion, and trace index pinned at lru=128. On projects above 128 files those per-file memos are evicted during the build, so a revision bump makes salsa revalidation recompute them instead of consulting their memo headers. Wire all four into the capacity setter and raise DEFAULT_PARSE_LRU_CAP to 1024. The incremental semantic-index rebuild's revalidation pass drops accordingly (unchanged-file revalidation ~170ms -> ~70ms per file in common_cells).
source_root_semantic_index merged reference groups and module call edges in one query, so a find-references or rename request revalidated both. Split it into source_root_reference_index and source_root_module_edge_index so each request only pays for the half it needs. common_cells references rebuild after one-file edit: ~5.3s -> ~3.9s.
The merged reference index was a salsa query aggregating every file index, so a revision bump made salsa deep-verify all files (~2.5s on common_cells). Move the materialized index out of salsa into a RootDb-side cache that re-indexes only the files changed by apply_change. A changed file whose ItemTree is unchanged cannot have changed its cross-file-visible definitions, so the other files' indexes stay valid and are reused from the cache; a structural change conservatively falls back to a full rebuild. definition_ranges_for is also memoized per DefId so the incremental re-merge no longer re-projects every definition's origins. common_cells references rebuild after one-file edit: ~3.9s -> ~1.8s. The remaining cost is the monolithic parsed_profile re-parse of unchanged files.
parsed_compilation_unit pulled every root's tree from the monolithic parsed_profile, so editing one file re-parsed the whole profile. Parse roots standalone instead, injecting the running compilation-unit macro set of predecessor roots as predefines so cross-file `$unit` macro visibility is preserved without a conservative fallback. Salsa propagates precisely: a root whose own `$unit` macros change invalidates only the downstream roots, not the whole profile.
parsed_compilation_unit returned syntax tree and preprocessor trace together, so a syntax-only edit (a comment) revalidated the trace and, through the $unit macro chain, every downstream root. Split it into parse_tree and preproc_trace so the trace is a separate memo that backdates on comment edits. Verified with preproc-expand (73) and ide (203) test suites; the two profile-reuse tests were updated to assert the new standalone-parse contract.
The incremental path re-merged all cached file indexes through from_file_indexes, which re-projected definition origins for every definition on each rebuild. Patch the cached index in place: existing definitions keep their cached name and definition ranges, and only a dirty file's references are swapped. Profile: file_semantic_index re-read is the remaining rebuild cost; the merge itself is now ~4ms.
The nameres core read three O(project) globals per file: unit_scope, design_map and unit_index (plus the per-root module index in ide). Thread a precomputed ResolutionContext through resolve_name/resolve_path/ resolve_in_resolved_scopes and the ide slow path so the file index's salsa dependency graph no longer includes the whole-project globals. The index build computes the context once per revision and reuses it across files; non-index callers (goto-def, hover, completion, hints) compute it once per request. No fallback: every caller supplies the context explicitly.
L0 UnitId is a name, not identity. Projecting it onto OwnerId by name is why GeneratedUnits existed and why a stale overlay could send goto to a module that was no longer there. Resolution now uses the source catalog only as a file locator and reads OwnerId from the salsa owner table, including HirFileId::Macro for paid files. Generated names cannot linger after an edit because the owner table is the parse.
A live compilation has thousands of symbols. Comparing a path string at each one was cheap only on the T4 single-file slice.
The T4 slice built a Compilation per request, fed one file, and treated missing as None because hir-ty was still the fallback. A service that can replace hir-ty has to name the snapshot it answered and say when it could not.
hir-ty had a test that made ClassDef look like type-system work. The record is names and member kinds; slang answers inheritance.
Exact freshness match dropped a second-long analysis on the first keystroke. The old test asserted that defect. Results now reproject and say how many edits they predate.
HierPath is the hub anchor for instance-level backends. The live compilation already has the tree; this just names it.
SourceSession disables proximate paths, so getRawFileName is only the basename. Lookup and instance listing used that display name, which is why FileId mapping had to guess by filename.
A Definition anchor cannot name an elaborated instance. Vide stores HierPath; the live compilation reprojects the instantiation site.
Hover type display and class :: need the elaborated symbol at a caret, not only class members at their declarations.
TypeSystem on hover printed unknown for Unresolved and never had class. The live compilation already has the type. Class :: was waiting for a lowering path that will not exist.
The resolver said package/class :: waited on type lowering. Package :: is export-scope names. Class :: is the elaboration service.
Duplicate packages are one slang name, not an HIR Ambiguous pair. pathres keeps hierarchical dots only.
Offset walk cannot see a hierarchical prefix such as top.u0. The smallest span inside a selection is an operand, not the selected expression.
`.` needs members of the prefix, including hierarchical instances. Expected-type filtering compared two HIR types; slang is the authority, so the filter is dropped rather than rebuilt.
A missing slang type is not logic. Mixed-width b + a must insert the sum, not the first operand.
Production ide no longer constructs TypeSystem. HirDisplay of lowered syntax stays for render and signature help.
The lint job failed on wrapping, not on behavior.
Class-member lookup is test-only now. Unused i18n keys and collapsible ifs fail the CI lint job.
Slang joins parent directory with backslash and the include literal with forward slashes. Unix /rtl is not an include directory on Windows.
Scope::lookupName asserts empty selectors. A completion prefix like bus[0] is a real input, not a hierarchical name.
Width-incompatible names are offered again. Extract-variable over LSP needs a compilation profile so slang can type the expression.
Declaration render already has the type. Slang only fills hover when HIR has nothing, without a labeled block.
Three changes that cannot be separated without leaving the tree unbuildable. One pipeline. Seven request variants each carried their own channel plumbing: the same recv_timeout arms, the same catch_unwind, the same generation lookup, written out seven times. Six of them also called handle_lookup with dummy arguments purely to trigger the rebuild, then threw its answer away and redid the lookup. Reaching the live compilation is one step, so it is one function; a query is a closure the worker runs. Two impossible-state branches that returned NotReady go with it: the generation exists because the caller just built it, and a missing profile is OutsideAnyProfile, which says waiting will not help. Cancellation is not a crash. Salsa unwinds through rebuild whenever the workspace moves on. That stored a poisoned generation and reported Crashed for that revision forever. Cancelled::catch names it and re-raises everything else, so a Rust bug kills the worker instead of hiding. The class-member path is deleted, not kept. SymbolInfo is a superset of ClassMemberInfo, so lookup_class_member had already become dead weight held alive by tests: its Rust, FFI, and C++ sides were all reachable only under cfg(test). Its two tests now drive the shipped offset entry, and t4_gate_numbers loses the or_else that let it pass by opening a private single-file compilation when the shipped path answered nothing. The request path no longer waits out a cold elaboration. The prewarm already held the service handle but never asked it to build, so the first request paid for the whole thing on the keyboard path behind a 60s timeout. Prewarm builds it; the request path gives up after 150ms and the caller keeps the HIR answer.
Two things the ElabResult contract was supposed to prevent, and did not. Every consumer threw the contract away. Seven call sites each wrote their own `Ready(Some(x)) else return None`, so Stale, NotReady, Cancelled, Crashed and WorkerGone all arrived at the user as "no such symbol", and the service had no tracing at all. `answered` is now the single way to turn the enum into an Option: routine degradation is debug, a crash or a dead worker is warn. Falling back to HIR is still right; doing it silently was not. Cross-generation SyntaxTree reuse aborted the process. A Compilation constructs its own SourceSession, every tree belongs to the session that parsed it, and add_syntax_tree throws std::logic_error on a foreign one. The one test covering reuse passed only because it reused the first root, which adopts the old session before any parse; wiring the prewarm made every second generation take the path and SIGABRT the suite. Reuse needs a session outliving one generation plus SourceManager::replaceBuffer, which is slang-sys work, so the machinery is gone rather than left as a trap, with the reason recorded where it was. The test now asserts what is actually observable: an unrelated edit does not change another root's answer.
… assert `Scope::lookupName` ends in `SLANG_ASSERT(result.selectors.empty())` and the wrapper caught the resulting AssertionException, on the theory that the assert means "the name had selectors". It does not. `u0[0]` on an instance array resolves and leaves no selectors; the assert fires when a select could not be applied, as on `bus[0]`. Which case a completion prefix falls into is not knowable before the lookup, so no caller-side check can establish the precondition — the wrapper was catching a programmer-error assert to ask a question it never asked. `Lookup::name` is the API that answers it: it fills a LookupResult, and leftover `selectors` is the same signal without the assert. One helper reads it, and the catch is gone. Two of the four lookup strategies go with this. `find_named_symbol` tried package, root name, class, then a design walk, and no caller could say which had answered; it is now `find_named_scope` with the namespaces named and the walk documented for what it is — the only route to a name inside an instance body when the buffer being completed in does not parse, so there is no expression to resolve instead. `lookup_scoped` had its own copy of the package-then-class half; it calls the shared one now. Tests wait for the prewarm. A request that lands before the elaboration does answers from HIR, which is right in an editor and makes a snapshot depend on which one won.
Handoff notes and perf baselines stay on disk; they are not part of the branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.