Skip to content

Commit

Permalink
Remove some unnecessary string allocations in hosting layer (dotnet#9…
Browse files Browse the repository at this point in the history
…5801)

- Make utils for ends_with and starts_with stop requiring a string
- Avoid creating a string for tracing when tracing is not enabled

This removes ~2000 transient string allocations in starting a console app.
  • Loading branch information
elinor-fung authored Jan 4, 2024
1 parent 2242665 commit 6059d9f
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/native/corehost/fxr/command_line.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ namespace

app_candidate = argv[*new_argoff];

bool is_app_managed = ends_with(app_candidate, _X(".dll"), false) || ends_with(app_candidate, _X(".exe"), false);
bool is_app_managed = utils::ends_with(app_candidate, _X(".dll"), false) || utils::ends_with(app_candidate, _X(".exe"), false);
if (!is_app_managed)
{
trace::verbose(_X("Application '%s' is not a managed executable."), app_candidate.c_str());
Expand Down
6 changes: 3 additions & 3 deletions src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ namespace

// Look up the root package instead of the "runtime" package because we can't do a full rid resolution.
// i.e., look for "Microsoft.NETCore.DotNetHostPolicy/" followed by version.
pal::string_t prefix = _X("Microsoft.NETCore.DotNetHostPolicy/");
const pal::char_t prefix[] = _X("Microsoft.NETCore.DotNetHostPolicy/");
for (const auto& library : json.document()[_X("libraries")].GetObject())
{
pal::string_t lib_name{library.name.GetString()};
if (starts_with(lib_name, prefix, false))
if (utils::starts_with(lib_name, prefix, false))
{
// Extract the version information that occurs after '/'
retval = lib_name.substr(prefix.size());
retval = lib_name.substr(utils::strlen(prefix));
break;
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/native/corehost/hostmisc/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#define xerr std::wcerr
#define xout std::wcout
#define DIR_SEPARATOR L'\\'
#define DIR_SEPARATOR_STR L"\\"
#define PATH_SEPARATOR L';'
#define PATH_MAX MAX_PATH
#define _X(s) L ## s
Expand All @@ -44,6 +45,7 @@
#define xerr std::cerr
#define xout std::cout
#define DIR_SEPARATOR '/'
#define DIR_SEPARATOR_STR "/"
#define PATH_SEPARATOR ':'
#undef _X
#define _X(s) s
Expand Down Expand Up @@ -143,7 +145,7 @@ namespace pal
CRITICAL_SECTION _impl;
};

inline string_t exe_suffix() { return _X(".exe"); }
inline const pal::char_t* exe_suffix() { return _X(".exe"); }

inline int cstrcasecmp(const char* str1, const char* str2) { return ::_stricmp(str1, str2); }
inline int strcmp(const char_t* str1, const char_t* str2) { return ::wcscmp(str1, str2); }
Expand Down Expand Up @@ -211,7 +213,7 @@ namespace pal
typedef void* proc_t;
typedef std::mutex mutex_t;

inline string_t exe_suffix() { return _X(""); }
inline const pal::char_t* exe_suffix() { return nullptr; }

inline int cstrcasecmp(const char* str1, const char* str2) { return ::strcasecmp(str1, str2); }
inline int strcmp(const char_t* str1, const char_t* str2) { return ::strcmp(str1, str2); }
Expand Down
24 changes: 8 additions & 16 deletions src/native/corehost/hostmisc/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,6 @@ bool utils::ends_with(const pal::string_t& value, const pal::char_t* suffix, siz
cmp(value.c_str() + value.size() - suffix_len, suffix) == 0;
}

bool ends_with(const pal::string_t& value, const pal::string_t& suffix, bool match_case)
{
return utils::ends_with(value, suffix.c_str(), suffix.size(), match_case);
}

bool starts_with(const pal::string_t& value, const pal::string_t& prefix, bool match_case)
{
return utils::starts_with(value, prefix.c_str(), prefix.size(), match_case);
}

void append_path(pal::string_t* path1, const pal::char_t* path2)
{
if (pal::is_path_rooted(path2))
Expand All @@ -78,17 +68,19 @@ void append_path(pal::string_t* path1, const pal::char_t* path2)

pal::string_t strip_executable_ext(const pal::string_t& filename)
{
pal::string_t exe_suffix = pal::exe_suffix();
if (exe_suffix.empty())
{
const pal::char_t* exe_suffix = pal::exe_suffix();
if (exe_suffix == nullptr)
return filename;

size_t suffix_len = pal::strlen(exe_suffix);
if (suffix_len == 0)
return filename;
}

if (ends_with(filename, exe_suffix, false))
if (utils::ends_with(filename, exe_suffix, suffix_len, false))
{
// We need to strip off the old extension
pal::string_t result(filename);
result.erase(result.size() - exe_suffix.size());
result.erase(result.size() - suffix_len);
return result;
}

Expand Down
3 changes: 0 additions & 3 deletions src/native/corehost/hostmisc/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@

#define HOST_VERSION _QUOTE(RuntimeProductVersion)

bool ends_with(const pal::string_t& value, const pal::string_t& suffix, bool match_case);
bool starts_with(const pal::string_t& value, const pal::string_t& prefix, bool match_case);

namespace utils
{
template<size_t L>
Expand Down
4 changes: 2 additions & 2 deletions src/native/corehost/hostpolicy/deps_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void deps_json_t::reconcile_libraries_with_targets(
for (const auto& asset : assets)
{
auto asset_name = asset.name;
if (ends_with(asset_name, _X(".ni"), false))
if (utils::ends_with(asset_name, _X(".ni"), false))
{
asset_name = strip_file_ext(asset_name);
}
Expand Down Expand Up @@ -595,7 +595,7 @@ void deps_json_t::load(bool is_framework_dependent, std::function<void(const jso
runtime_target.GetString() :
runtime_target[_X("name")].GetString();

trace::verbose(_X("Loading deps file... [%s] as framework dependent=%d, use_fallback_graph=%d"), m_deps_file.c_str(), is_framework_dependent, m_rid_resolution_options.use_fallback_graph);
trace::verbose(_X("Loading deps file... [%s]: is_framework_dependent=%d, use_fallback_graph=%d"), m_deps_file.c_str(), is_framework_dependent, m_rid_resolution_options.use_fallback_graph);

if (is_framework_dependent)
{
Expand Down
15 changes: 8 additions & 7 deletions src/native/corehost/hostpolicy/deps_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace

trace::verbose(_X("Adding to %s path: %s"), deps_entry_t::s_known_asset_types[asset_type], path.c_str());

if (starts_with(path, svc_dir, false))
if (utils::starts_with(path, svc_dir.c_str(), svc_dir.length(), false))
{
serviced->append(path);
serviced->push_back(PATH_SEPARATOR);
Expand Down Expand Up @@ -280,7 +280,8 @@ bool deps_resolver_t::probe_deps_entry(const deps_entry_t& entry, const pal::str

for (const auto& config : m_probes)
{
trace::verbose(_X(" Using probe config: %s"), config.as_str().c_str());
if (trace::is_enabled())
trace::verbose(_X(" Using probe config: %s"), config.as_str().c_str());

if (config.is_servicing() && !entry.is_serviceable)
{
Expand Down Expand Up @@ -425,7 +426,7 @@ bool deps_resolver_t::resolve_tpa_list(
}

// Ignore placeholders
if (ends_with(entry.asset.relative_path, _X("/_._"), false))
if (utils::ends_with(entry.asset.relative_path, _X("/_._"), false))
{
return true;
}
Expand Down Expand Up @@ -601,7 +602,7 @@ void deps_resolver_t::init_known_entry_path(const deps_entry_t& entry, const pal
}

assert(pal::is_path_rooted(path));
if (m_coreclr_path.empty() && ends_with(path, DIR_SEPARATOR + pal::string_t(LIBCORECLR_NAME), false))
if (m_coreclr_path.empty() && utils::ends_with(path, DIR_SEPARATOR_STR LIBCORECLR_NAME, false))
{
m_coreclr_path = path;
return;
Expand Down Expand Up @@ -639,7 +640,7 @@ void deps_resolver_t::resolve_additional_deps(const pal::char_t* additional_deps
while (std::getline(ss, additional_deps_path, PATH_SEPARATOR))
{
// If it's a single deps file, insert it in 'm_additional_deps_files'
if (ends_with(additional_deps_path, _X(".deps.json"), false))
if (utils::ends_with(additional_deps_path, _X(".deps.json"), false))
{
if (pal::file_exists(additional_deps_path))
{
Expand Down Expand Up @@ -787,7 +788,7 @@ bool deps_resolver_t::resolve_probe_dirs(
}

// Ignore placeholders
if (ends_with(entry.asset.relative_path, _X("/_._"), false))
if (utils::ends_with(entry.asset.relative_path, _X("/_._"), false))
{
return true;
}
Expand All @@ -808,7 +809,7 @@ bool deps_resolver_t::resolve_probe_dirs(
{
// For self-contained apps do not use the full package name
// because of rid-fallback could happen (ex: CentOS falling back to RHEL)
if ((entry.asset.name == _X("apphost")) && ends_with(entry.library_name, _X(".Microsoft.NETCore.DotNetAppHost"), false))
if ((entry.asset.name == _X("apphost")) && utils::ends_with(entry.library_name, _X(".Microsoft.NETCore.DotNetAppHost"), false))
{
return report_missing_assembly_in_manifest(entry, true);
}
Expand Down
6 changes: 3 additions & 3 deletions src/native/corehost/hostpolicy/hostpolicy_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,9 @@ int hostpolicy_context_t::initialize(const hostpolicy_init_t &hostpolicy_init, c
}

host_contract.get_runtime_property = &get_runtime_property;
pal::stringstream_t ptr_stream;
ptr_stream << "0x" << std::hex << (size_t)(&host_contract);
if (!coreclr_properties.add(_STRINGIFY(HOST_PROPERTY_RUNTIME_CONTRACT), ptr_stream.str().c_str()))
pal::char_t buffer[STRING_LENGTH("0xffffffffffffffff")];
pal::snwprintf(buffer, ARRAY_SIZE(buffer), _X("0x%zx"), (size_t)(&host_contract));
if (!coreclr_properties.add(_STRINGIFY(HOST_PROPERTY_RUNTIME_CONTRACT), buffer))
{
log_duplicate_property_error(_STRINGIFY(HOST_PROPERTY_RUNTIME_CONTRACT));
return StatusCode::LibHostDuplicateProperty;
Expand Down

0 comments on commit 6059d9f

Please sign in to comment.