Reduce the amount of string copying and comparing when resolving assets on startup#123919
Reduce the amount of string copying and comparing when resolving assets on startup#123919elinor-fung merged 4 commits intodotnet:mainfrom
Conversation
Change probe_deps_entry to return a probe_result_t enum indicating: - not_found: entry was not found - bundled: entry was found in the single-file bundle - serviced: entry was found in a servicing location - found: entry was found in a regular location Update add_unique_path to take a bool is_serviced parameter instead of comparing the path against the servicing directory string, using the enum value from probe_deps_entry to determine serviced vs non-serviced.
- Store const deps_asset_t* instead of deps_asset_t in deps_resolved_asset_t - Move resolved_path instead of copying - Add assembly_version() and file_version() helper methods to encapsulate null handling - Pass nullptr for entries from get_dir_assemblies (no version info needed) - Add version_t::empty() static method to get a reusable empty version - Remove parameterless version_t constructor
|
Tagging subscribers to this area: @agocke, @jeffschwMSFT, @elinor-fung |
There was a problem hiding this comment.
Pull request overview
This PR optimizes corehost startup asset resolution by reducing per-asset string work and avoiding copying deps_asset_t entries when building the TPA list.
Changes:
- Introduces
version_t::empty()and removes the defaultversion_t()constructor to avoid repeated construction of “empty” versions. - Refactors TPA resolution to store a pointer to
deps_asset_t(instead of copying the full asset) and moves resolved path strings. - Replaces per-path “is servicing” prefix checks with a probe result classification (
probe_result_t) returned byprobe_deps_entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/corehost/hostpolicy/version.h | Removes default ctor declaration; adds version_t::empty() API. |
| src/native/corehost/hostpolicy/version.cpp | Implements version_t::empty() as a shared static empty instance. |
| src/native/corehost/hostpolicy/deps_resolver.h | Introduces probe_result_t; changes resolved-asset representation to store deps_asset_t*. |
| src/native/corehost/hostpolicy/deps_resolver.cpp | Updates probing and TPA/probe-dir resolution logic to use probe_result_t and avoid asset copies. |
| src/native/corehost/hostpolicy/deps_format.cpp | Initializes version fields using version_t::empty() instead of default construction. |
| src/native/corehost/hostpolicy/deps_entry.h | Updates deps_asset_t default ctor to use version_t::empty(). |
58180cc to
e4ff9df
Compare
|
Totally a blue-sky thought: imagining a managed corehost replacement implemented as an AOT’d host (e.g. under All currently completed runtime ports already support AOT. The few community platforms in progress (haiku and illumos) can keep using |
In the past, I've generally thought it not worth the investment given the expected decent size increase. However, some AI-assisted prototyping to see what it would actually look like could be interesting. |
deps_asset_tasset when building the TPA listThis removes ~1000 allocations for startup of an empty console app (linear based on the number of assemblies the app depends on).
Local runs of staticconsoletemplate on Windows indicated a difference that was within measurement noise range, but it was consistent across 10 runs of 10 iterations each - combined summary:
cc @dotnet/appmodel @AaronRobinsonMSFT