Skip to content

Reduce file existence checks when servicing directory exists #123568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
elinor-fung wants to merge 1 commit into
base: main
Choose a base branch
from
+6 −3
Open

Reduce file existence checks when servicing directory exists #123568

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/native/corehost/hostpolicy/deps_resolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,6 @@ void deps_resolver_t::setup_probe_config(
pal::string_t ext_pkgs = m_core_servicing;
append_path(&ext_pkgs, _X("pkgs"));
m_probes.push_back(probe_config_t::svc(ext_pkgs));

m_needs_file_existence_checks = true;
}

// The published deps directory to be probed: either app or FX directory.
Expand Down Expand Up @@ -354,7 +352,12 @@ bool deps_resolver_t::probe_deps_entry(const deps_entry_t& entry, const pal::str
}
else
{
if (entry.to_library_package_path(config.probe_dir, candidate, search_options | (config.is_servicing() ? deps_entry_t::search_options::is_servicing : 0)))
if (config.is_servicing())
{
search_options |= deps_entry_t::search_options::is_servicing | deps_entry_t::search_options::file_existence;
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Jan 23, 2026

Choose a reason for hiding this comment

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

Why is the file_existence flag needed now? Was it added elsewhere previously?

Copy link
Member Author

@elinor-fung elinor-fung Jan 23, 2026

Choose a reason for hiding this comment

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

It is also added if additional probe paths or a shared store is specified (m_needs_file_existence_check). In those cases, it is added for all probe configs.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Jan 24, 2026

Choose a reason for hiding this comment

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

So what you're saying is setting m_needs_file_existence_checks to true sets the file_existence flag and we could technically assert the old code that that flag was set? If so, does is_servicing imply file_existence?

Copy link
Member Author

@elinor-fung elinor-fung Jan 24, 2026

Choose a reason for hiding this comment

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

setting m_needs_file_existence_checks to true sets the file_existence flag

Yes. We initialize search_options based on m_needs_file_existence_checks.

we could technically assert the old code that that flag was set

Do you mean assert that m_needs_file_existence_checks is the same as whether search_options has the file_existence flag? Then yes. But it is initialized exactly to a couple lines above in the same function, so I don't know that it would be useful.

does is_servicing imply file_existence?

Yes. I do think is_servicing could go away - it doesn't seem so much a search option as just added in there specifically for a single-file scenario, which I think we could do differently.

}

if (entry.to_library_package_path(config.probe_dir, candidate, search_options))
{
trace::verbose(_X(" Probed package dir and matched '%s'"), candidate->c_str());
return true;
Expand Down
Loading