From bc6c7339f9a45cfdf15a5a2e941b9b4694c75589 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 2 Mar 2023 11:21:16 -0800 Subject: [PATCH] Stop storing the parsed RID fallback graph (#82811) After startup, we only need the RID fallback graph from the root framework for component dependency resolution (AssemblyDependencyResolver). This change stops storing that graph (which is decently large for portable linux) for all applications and switches to re-populating it on demand. Removing this stored value also lines up with the plan to move to algorithmic RID selection rather than reading from the .deps.json. --- .../StandaloneAppActivation.cs | 2 +- src/native/corehost/hostpolicy/deps_format.cpp | 97 ++++++++++++++-------- src/native/corehost/hostpolicy/deps_format.h | 5 ++ src/native/corehost/hostpolicy/deps_resolver.h | 18 ++-- src/native/corehost/hostpolicy/hostpolicy.cpp | 49 ++++++++--- .../corehost/hostpolicy/hostpolicy_context.cpp | 6 +- .../corehost/hostpolicy/hostpolicy_context.h | 2 +- src/native/corehost/hostpolicy/hostpolicy_init.h | 2 - 8 files changed, 120 insertions(+), 61 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs b/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs index d76559c..3559f47 100644 --- a/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs @@ -85,7 +85,7 @@ namespace HostActivation.Tests .Should().Pass() .And.HaveStdOut($"Hello World!{Environment.NewLine}{Environment.NewLine}.NET {sharedTestState.RepoDirectories.MicrosoftNETCoreAppVersion}{Environment.NewLine}") .And.HaveStdErrContaining($"Runtime config does not exist at [{fixture.TestProject.RuntimeConfigJson}]") - .And.HaveStdErrContaining($"Could not locate the dependencies manifest file [{fixture.TestProject.DepsJson}]"); + .And.HaveStdErrContaining($"Dependencies manifest does not exist at [{fixture.TestProject.DepsJson}]"); } [Fact] diff --git a/src/native/corehost/hostpolicy/deps_format.cpp b/src/native/corehost/hostpolicy/deps_format.cpp index f9ffd4f..739cd04 100644 --- a/src/native/corehost/hostpolicy/deps_format.cpp +++ b/src/native/corehost/hostpolicy/deps_format.cpp @@ -39,6 +39,65 @@ namespace return path; } + + void populate_rid_fallback_graph(const json_parser_t::value_t& json, deps_json_t::rid_fallback_graph_t& rid_fallback_graph) + { + const auto& json_object = json.GetObject(); + if (json_object.HasMember(_X("runtimes"))) + { + for (const auto& rid : json[_X("runtimes")].GetObject()) + { + auto& vec = rid_fallback_graph[rid.name.GetString()]; + const auto& fallback_array = rid.value.GetArray(); + vec.reserve(fallback_array.Size()); + for (const auto& fallback : fallback_array) + { + vec.push_back(fallback.GetString()); + } + } + } + + if (trace::is_enabled()) + { + trace::verbose(_X("The rid fallback graph is: {")); + for (const auto& rid : rid_fallback_graph) + { + trace::verbose(_X("%s => ["), rid.first.c_str()); + for (const auto& fallback : rid.second) + { + trace::verbose(_X("%s, "), fallback.c_str()); + } + trace::verbose(_X("]")); + } + trace::verbose(_X("}")); + } + } + + bool deps_file_exists(pal::string_t& deps_path) + { + if (bundle::info_t::config_t::probe(deps_path) || pal::realpath(&deps_path, /*skip_error_logging*/ true)) + return true; + + trace::verbose(_X("Dependencies manifest does not exist at [%s]"), deps_path.c_str()); + return false; + } +} + +deps_json_t::rid_fallback_graph_t deps_json_t::get_rid_fallback_graph(const pal::string_t& deps_path) +{ + rid_fallback_graph_t rid_fallback_graph; + trace::verbose(_X("Getting RID fallback graph for deps file... %s"), deps_path.c_str()); + + pal::string_t deps_path_local = deps_path; + if (!deps_file_exists(deps_path_local)) + return rid_fallback_graph; + + json_parser_t json; + if (!json.parse_file(deps_path_local)) + return rid_fallback_graph; + + populate_rid_fallback_graph(json.document(), rid_fallback_graph); + return rid_fallback_graph; } void deps_json_t::reconcile_libraries_with_targets( @@ -367,36 +426,7 @@ void deps_json_t::load_self_contained(const pal::string_t& deps_path, const json }; reconcile_libraries_with_targets(deps_path, json, package_exists, get_relpaths); - - const auto& json_object = json.GetObject(); - if (json_object.HasMember(_X("runtimes"))) - { - for (const auto& rid : json[_X("runtimes")].GetObject()) - { - auto& vec = m_rid_fallback_graph[rid.name.GetString()]; - const auto& fallback_array = rid.value.GetArray(); - vec.reserve(fallback_array.Size()); - for (const auto& fallback : fallback_array) - { - vec.push_back(fallback.GetString()); - } - } - } - - if (trace::is_enabled()) - { - trace::verbose(_X("The rid fallback graph is: {")); - for (const auto& rid : m_rid_fallback_graph) - { - trace::verbose(_X("%s => ["), rid.first.c_str()); - for (const auto& fallback : rid.second) - { - trace::verbose(_X("%s, "), fallback.c_str()); - } - trace::verbose(_X("]")); - } - trace::verbose(_X("}")); - } + populate_rid_fallback_graph(json, m_rid_fallback_graph); } bool deps_json_t::has_package(const pal::string_t& name, const pal::string_t& ver) const @@ -429,17 +459,16 @@ bool deps_json_t::has_package(const pal::string_t& name, const pal::string_t& ve void deps_json_t::load(bool is_framework_dependent, const pal::string_t& deps_path, const rid_fallback_graph_t& rid_fallback_graph) { m_deps_file = deps_path; - m_file_exists = bundle::info_t::config_t::probe(deps_path) || pal::realpath(&m_deps_file, true); + m_file_exists = deps_file_exists(m_deps_file); - json_parser_t json; if (!m_file_exists) { - // If file doesn't exist, then assume parsed. - trace::verbose(_X("Could not locate the dependencies manifest file [%s]. Some libraries may fail to resolve."), deps_path.c_str()); + // Not existing is valid m_valid = true; return; } + json_parser_t json; if (!json.parse_file(m_deps_file)) return; diff --git a/src/native/corehost/hostpolicy/deps_format.h b/src/native/corehost/hostpolicy/deps_format.h index 52348d3..b152129 100644 --- a/src/native/corehost/hostpolicy/deps_format.h +++ b/src/native/corehost/hostpolicy/deps_format.h @@ -61,6 +61,11 @@ public: return m_deps_file; } +public: // static + // Get the RID fallback graph for a deps file. + // Parse failures or non-existent files will return an empty fallback graph + static rid_fallback_graph_t get_rid_fallback_graph(const pal::string_t& deps_path); + private: void load_self_contained(const pal::string_t& deps_path, const json_parser_t::value_t& json, const pal::string_t& target_name); void load_framework_dependent(const pal::string_t& deps_path, const json_parser_t::value_t& json, const pal::string_t& target_name, const rid_fallback_graph_t& rid_fallback_graph); diff --git a/src/native/corehost/hostpolicy/deps_resolver.h b/src/native/corehost/hostpolicy/deps_resolver.h index 53f6c38..1d32e10 100644 --- a/src/native/corehost/hostpolicy/deps_resolver.h +++ b/src/native/corehost/hostpolicy/deps_resolver.h @@ -173,6 +173,15 @@ public: } } +public: // static + static pal::string_t get_fx_deps(const pal::string_t& fx_dir, const pal::string_t& fx_name) + { + pal::string_t fx_deps = fx_dir; + pal::string_t fx_deps_name = fx_name + _X(".deps.json"); + append_path(&fx_deps, fx_deps_name.c_str()); + return fx_deps; + } + private: void setup_shared_store_probes( const std::vector& shared_stores); @@ -194,14 +203,7 @@ private: return *m_fx_deps[0]; } - static pal::string_t get_fx_deps(const pal::string_t& fx_dir, const pal::string_t& fx_name) - { - pal::string_t fx_deps = fx_dir; - pal::string_t fx_deps_name = fx_name + _X(".deps.json"); - append_path(&fx_deps, fx_deps_name.c_str()); - return fx_deps; - } - +private: // Resolve order for TPA lookup. bool resolve_tpa_list( pal::string_t* output, diff --git a/src/native/corehost/hostpolicy/hostpolicy.cpp b/src/native/corehost/hostpolicy/hostpolicy.cpp index 83e5f25..440ae4f 100644 --- a/src/native/corehost/hostpolicy/hostpolicy.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy.cpp @@ -97,7 +97,7 @@ namespace } int create_hostpolicy_context( - hostpolicy_init_t &hostpolicy_init, + const hostpolicy_init_t &hostpolicy_init, const int argc, const pal::char_t *argv[], bool breadcrumbs_enabled, @@ -831,6 +831,33 @@ SHARED_API int HOSTPOLICY_CALLTYPE corehost_unload() return StatusCode::Success; } +namespace +{ + pal::string_t get_root_deps_file(const hostpolicy_init_t& init) + { + if (init.is_framework_dependent) + { + const fx_definition_t& root_fx = get_root_framework(init.fx_definitions); + return deps_resolver_t::get_fx_deps(root_fx.get_dir(), root_fx.get_name()); + } + + // For self-contained, the root deps file is the app's deps file + if (!init.deps_file.empty()) + return init.deps_file; + + const std::shared_ptr context = get_hostpolicy_context(/*require_runtime*/ true); + if (bundle::info_t::is_single_file_bundle()) + { + const bundle::runner_t* app = bundle::runner_t::app(); + const pal::string_t& app_root = app->base_path(); + + return get_deps_from_app_binary(app->base_path(), context->application); + } + + return get_deps_from_app_binary(get_directory(context->application), context->application); + } +} + SHARED_API int HOSTPOLICY_CALLTYPE corehost_resolve_component_dependencies( const pal::char_t *component_main_assembly_path, corehost_resolve_component_dependencies_result_fn result) @@ -850,20 +877,19 @@ SHARED_API int HOSTPOLICY_CALLTYPE corehost_resolve_component_dependencies( // IMPORTANT: g_init is static/global and thus potentially accessed from multiple threads // We must only use it as read-only here (unlike the run scenarios which own it). - // For example the frameworks in g_init.fx_definitions can't be used "as-is" by the resolver - // right now as it would try to re-parse the .deps.json and thus modify the objects. + const hostpolicy_init_t &init = g_init; // The assumption is that component dependency resolution will only be called // when the coreclr is hosted through this hostpolicy and thus it will // have already called corehost_main_init. - if (!g_init.host_info.is_valid(g_init.host_mode)) + if (!init.host_info.is_valid(init.host_mode)) { trace::error(_X("Hostpolicy must be initialized and corehost_main must have been called before calling corehost_resolve_component_dependencies.")); return StatusCode::CoreHostLibLoadFailure; } // If the current host mode is libhost, use apphost instead. - host_mode_t host_mode = g_init.host_mode == host_mode_t::libhost ? host_mode_t::apphost : g_init.host_mode; + host_mode_t host_mode = init.host_mode == host_mode_t::libhost ? host_mode_t::apphost : init.host_mode; // Initialize arguments (basically the structure describing the input app/component to resolve) arguments_t args; @@ -906,15 +932,18 @@ SHARED_API int HOSTPOLICY_CALLTYPE corehost_resolve_component_dependencies( // TODO Review: Since we're only passing the one component framework, the resolver will not consider // frameworks from the app for probing paths. So potential references to paths inside frameworks will not resolve. - // The RID graph still has to come from the actual root framework, so take that from the g_init.root_rid_fallback_graph, - // which stores the fallback graph for the app's root framework. + // The RID graph still has to come from the actual root framework, so get the deps file + // for the app's root framework and read in the graph. + static deps_json_t::rid_fallback_graph_t root_rid_fallback_graph = + deps_json_t::get_rid_fallback_graph(get_root_deps_file(init)); + deps_resolver_t resolver( args, component_fx_definitions, /* additional_deps_serialized */ nullptr, // Additional deps - don't use those from the app, they're already in the app - shared_store::get_paths(g_init.tfm, host_mode, g_init.host_info.host_path), - g_init.probe_paths, - &g_init.root_rid_fallback_graph, + shared_store::get_paths(init.tfm, host_mode, init.host_info.host_path), + init.probe_paths, + &root_rid_fallback_graph, true); pal::string_t resolver_errors; diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 11fc788..cc0bc21 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -135,7 +135,7 @@ namespace } } -int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const arguments_t &args, bool enable_breadcrumbs) +int hostpolicy_context_t::initialize(const hostpolicy_init_t &hostpolicy_init, const arguments_t &args, bool enable_breadcrumbs) { application = args.managed_application; host_mode = hostpolicy_init.host_mode; @@ -160,10 +160,6 @@ int hostpolicy_context_t::initialize(hostpolicy_init_t &hostpolicy_init, const a return StatusCode::ResolverInitFailure; } - // Store the root framework's rid fallback graph so that we can - // use it for future dependency resolutions - hostpolicy_init.root_rid_fallback_graph = resolver.get_root_deps().get_rid_fallback_graph(); - probe_paths_t probe_paths; // Setup breadcrumbs. diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.h b/src/native/corehost/hostpolicy/hostpolicy_context.h index bec4630..608a637 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.h +++ b/src/native/corehost/hostpolicy/hostpolicy_context.h @@ -30,7 +30,7 @@ public: host_runtime_contract host_contract; - int initialize(hostpolicy_init_t &hostpolicy_init, const arguments_t &args, bool enable_breadcrumbs); + int initialize(const hostpolicy_init_t &hostpolicy_init, const arguments_t &args, bool enable_breadcrumbs); }; #endif // __HOSTPOLICY_CONTEXT_H__ diff --git a/src/native/corehost/hostpolicy/hostpolicy_init.h b/src/native/corehost/hostpolicy/hostpolicy_init.h index 590f010..5b0aeaf 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_init.h +++ b/src/native/corehost/hostpolicy/hostpolicy_init.h @@ -25,8 +25,6 @@ struct hostpolicy_init_t pal::string_t host_command; host_startup_info_t host_info; - std::unordered_map> root_rid_fallback_graph; - static bool init(const host_interface_t* input, hostpolicy_init_t* init); static void init_host_command(const host_interface_t* input, hostpolicy_init_t* init); -- 2.7.4