Skip to content

Reduce the amount of string copying and comparing when resolving assets on startup#123919

Merged
elinor-fung merged 4 commits intodotnet:mainfrom
elinor-fung:probe-deps-entry
Feb 5, 2026
Merged

Reduce the amount of string copying and comparing when resolving assets on startup#123919
elinor-fung merged 4 commits intodotnet:mainfrom
elinor-fung:probe-deps-entry

Conversation

@elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Feb 2, 2026

  • Avoid string comparison against servicing directory name for every asset (assembly, native, or resources)
  • Avoid copying every deps_asset_t asset when building the TPA list

This 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:

Metric Baseline Feature Delta
Average 55.77 ms 55.48 ms -0.29 ms (-0.5%)
Min 53.28 ms 53.14 ms
Max 57.29 ms 56.24 ms
StdDev 0.50 ms 0.35 ms
Rounds Won 0 10

cc @dotnet/appmodel @AaronRobinsonMSFT

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
Copilot AI review requested due to automatic review settings February 2, 2026 23:39
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke, @jeffschwMSFT, @elinor-fung
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 default version_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 by probe_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().

Copilot AI review requested due to automatic review settings February 3, 2026 20:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

@am11
Copy link
Member

am11 commented Feb 4, 2026

Totally a blue-sky thought: imagining a managed corehost replacement implemented as an AOT’d host (e.g. under src/native/managed)? Name suggestions corehostaot, nexthost or unihost. 🙂

All currently completed runtime ports already support AOT. The few community platforms in progress (haiku and illumos) can keep using corerun until their 'runtime port' is completed; which today effectively includes NativeAOT support.

@elinor-fung
Copy link
Member Author

Totally a blue-sky thought: imagining a managed corehost replacement implemented as an AOT’d host (e.g. under src/native/managed)?

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.

@elinor-fung elinor-fung merged commit 7cd3d5e into dotnet:main Feb 5, 2026
165 of 170 checks passed
@github-project-automation github-project-automation bot moved this to Done in AppModel Feb 5, 2026
@elinor-fung elinor-fung deleted the probe-deps-entry branch February 5, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants