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.
Lookup treated only HierarchyInstantiationSyntax.type as a definition. Checker and primitive instances use their own syntax, and named port labels live on the connection, not the port symbol name in the child.
Hierarchy comes from query_instances on a profile compile. Keystroke lookup still uses the file-closure compile and does not wait on that profile. UDP and named-port goto now bind through Compilation, so their navigation kinds are no longer HIR DefKind labels.
Open-file parse of a .svh include currently returns nothing because headers are not standalone parse units. P3.1 requires those diagnostics to come from the covering file-closure Compilation, mapped to the original FileId.
Include headers are not standalone parse units. Open-file parse now compiles the covering file closure and maps include-buffer diagnostics back to the original FileId, using the same Compiler seam as keystroke lookup. Profile semantic diagnostics stay on the background path.
Document diagnostics currently fall back to a full profile Compilation when the slang ledger is empty. P3.1 requires profile semantic diagnostics to come from the background compile, not the request path.
Document diagnostics now take parse from the covering file-closure Compilation and semantic diagnostics from the background ledger. The request path no longer compiles a full profile. Cached slang facts stay semantic-only so Vide and parse are merged at publish time, and stale ledger rows are not treated as current.
P3.1 requires mapping failures to stay visible through the single materializer. Unlocated slang diagnostics must not be dropped.
The materializer no longer drops slang diagnostics that have no source range. They stay visible on the mapped file as an empty file-level range instead of disappearing.
A diagnostic whose slang buffer is not in the compilation map is currently dropped. P3.1 requires that mapping failure to stay visible on the covering file.
Diagnostics whose slang buffer is not in this compilation stay visible on the covering file as file-level results. Their original buffer coordinates are not applied to a different FileId.
didClose after a profile compile must not duplicate Vide diagnostics. The compile and semantic-compiler paths must not mention process Command, which is the WASM diagnostic-path constraint.
P4.1 requires AnalysisHost::new not to start an elaboration worker, and request-path hover/goto must not wait on revision prewarm.
apply_change no longer spawns vide-revision-prewarm. Hover and goto compile on the calling thread, so request correctness does not wait on a background worker and Drop has no join protocol.
P4.2 requires the native BrowserServer/vide_lsp_message seam to answer package+user hover and goto on the calling thread, without a compiler worker or a blocking profile compile.
AnalysisHost::new is the only production constructor. The revision worker is gone, so the alias is dead. Record the WASM emscripten build script as the extra P4 link gate.
P5.1 requires UnitCatalog to be name → files. Duplicate CU names must surface as locator search hits, not a Unique/files[0] catalog binding.
UnitCatalog production lookups are name → files. Cursor classification and find-references/call-edges reconstruct this-file FileFacts hits from those files instead of treating catalog Unique UnitIds as identity.
P5.2 requires production hierarchy to come from Compilation. Catalog OwnerId projection of an instantiation type is the old HIR authority.
Cross-file hierarchy is Compilation. DefinitionClass keeps this-file lexical nameres and paid-parse HirFileId::Macro generated owners; it does not project catalog source files to a Unique OwnerId.
Production already locates by file. The Unique-style candidates list is dead. Lock AnalysisSnapshot/ProductStore against a third epoch clock: the store stays parse-deps, Compiler does not cache Compilation values.
nameres Type does not see compilation-unit modules. Keep this-file cu_owners_named_in_file so same-file rename/highlight survive. Cross-file best-effort rename used to reject via HIR catalog binding (ProjectScopeRequired); it now reports NoDefFound because that token is Compilation-owned.
Macro-generated modules must stay HirFileId::Macro so rename/highlight reject or reproject the call site. Source CU owners in this file remain the this-file lexical path after that miss.
Named ports are compilation names. Lock that pathres locate_hierarchy_targets must not Unique-bind a port declared in another file; goto still comes from Compilation.
pathres locate_hierarchy_targets was catalog → OwnerId binding for named ports, params, inlay, and completion. Cross-file hierarchy is Compilation; this-file CU owners and paid-parse generated owners stay for local nav. Old HIR Unique/Ambiguous snapshots of other-file modules are Unresolved.
Package import resolution belongs on DesignMap's export owners, not a pathres catalog name → OwnerId locator.
Delete pathres locate_packages. Import resolution matches names against the export-map owners already built for this revision, so pathres is this-file lexical again. Ambiguous packages stay Ambiguous.
pathres import resolution already looks up DesignMap owners. Reexport closure must not go back through catalog name → OwnerId.
Package export closure already has the source package OwnerIds. Reexport edges match those names instead of pathres catalog locate_cu_owners.
Instance `.` members and NamedType are compilation names. Lock that pathres instance descent and Type nameres do not Unique-bind them; goto still comes from Compilation.
Instance, interface, checker, and covergroup `.` members are Compilation. pathres still descends this-file blocks, generate, and clocking. `::` was already unresolved. This-file typedef Type nameres stays because lookup at the use is not a compilation symbol.
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.