Remove some unnecessary allocations / copies on startup (#59270)

diff --git a/src/coreclr/binder/inc/assemblyidentitycache.hpp b/src/coreclr/binder/inc/assemblyidentitycache.hpp
index bb70af1..86e70f4 100644 (file)
--- a/src/coreclr/binder/inc/assemblyidentitycache.hpp
+++ b/src/coreclr/binder/inc/assemblyidentitycache.hpp
@@ -16,7 +16,6 @@
 
 #include "bindertypes.hpp"
 #include "assemblyidentity.hpp"
-#include "utils.hpp"
 #include "sstring.h"
 #include "shash.h"
 

diff --git a/src/coreclr/binder/inc/utils.hpp b/src/coreclr/binder/inc/utils.hpp
index 768cf4f..24730b7 100644 (file)
--- a/src/coreclr/binder/inc/utils.hpp
+++ b/src/coreclr/binder/inc/utils.hpp
@@ -30,8 +30,8 @@ namespace BINDER_SPACE
 
     BOOL IsFileNotFound(HRESULT hr);
 
-    HRESULT GetNextPath(SString& paths, SString::Iterator& startPos, SString& outPath);
-    HRESULT GetNextTPAPath(SString& paths, SString::Iterator& startPos, bool dllOnly, SString& outPath, SString& simpleName, bool& isNativeImage);
+    HRESULT GetNextPath(const SString& paths, SString::CIterator& startPos, SString& outPath);
+    HRESULT GetNextTPAPath(const SString& paths, SString::CIterator& startPos, bool dllOnly, SString& outPath, SString& simpleName, bool& isNativeImage);
 };
 
 #endif

diff --git a/src/coreclr/binder/utils.cpp b/src/coreclr/binder/utils.cpp
index 2ae73b0..0dc7b7d 100644 (file)
--- a/src/coreclr/binder/utils.cpp
+++ b/src/coreclr/binder/utils.cpp
@@ -118,7 +118,7 @@ namespace BINDER_SPACE
         return RuntimeFileNotFound(hr);
     }
 
-    HRESULT GetNextPath(SString& paths, SString::Iterator& startPos, SString& outPath)
+    HRESULT GetNextPath(const SString& paths, SString::CIterator& startPos, SString& outPath)
     {
         HRESULT hr = S_OK;
 
@@ -140,8 +140,8 @@ namespace BINDER_SPACE
             wrappedWithQuotes = true;
         }
 
-        SString::Iterator iEnd = startPos;      // Where current path ends
-        SString::Iterator iNext;                // Where next path starts
+        SString::CIterator iEnd = startPos;      // Where current path ends
+        SString::CIterator iNext;                // Where next path starts
         if (wrappedWithQuotes)
         {
             if (paths.Find(iEnd, W('\"')))
@@ -186,7 +186,7 @@ namespace BINDER_SPACE
         return hr;
     }
 
-    HRESULT GetNextTPAPath(SString& paths, SString::Iterator& startPos, bool dllOnly, SString& outPath, SString& simpleName, bool& isNativeImage)
+    HRESULT GetNextTPAPath(const SString& paths, SString::CIterator& startPos, bool dllOnly, SString& outPath, SString& simpleName, bool& isNativeImage)
     {
         HRESULT hr = S_OK;
         isNativeImage = false;
@@ -205,7 +205,7 @@ namespace BINDER_SPACE
 
         {
             // Find the beginning of the simple name
-            SString::Iterator iSimpleNameStart = outPath.End();
+            SString::CIterator iSimpleNameStart = outPath.End();
 
             if (!outPath.FindBack(iSimpleNameStart, DIRECTORY_SEPARATOR_CHAR_W))
             {
@@ -222,11 +222,10 @@ namespace BINDER_SPACE
                 GO_WITH_HRESULT(E_INVALIDARG);
             }
 
-            // GCC complains if we create SStrings inline as part of a function call
-            SString sNiDll(W(".ni.dll"));
-            SString sNiExe(W(".ni.exe"));
-            SString sDll(W(".dll"));
-            SString sExe(W(".exe"));
+            const SString sNiDll(SString::Literal, W(".ni.dll"));
+            const SString sNiExe(SString::Literal, W(".ni.exe"));
+            const SString sDll(SString::Literal, W(".dll"));
+            const SString sExe(SString::Literal, W(".exe"));
 
             if (!dllOnly && (outPath.EndsWithCaseInsensitive(sNiDll) ||
                 outPath.EndsWithCaseInsensitive(sNiExe)))

diff --git a/src/coreclr/utilcode/longfilepathwrappers.cpp b/src/coreclr/utilcode/longfilepathwrappers.cpp
index 3afc611..d1372fa 100644 (file)
--- a/src/coreclr/utilcode/longfilepathwrappers.cpp
+++ b/src/coreclr/utilcode/longfilepathwrappers.cpp
@@ -21,16 +21,16 @@ private:
         static const WCHAR DirectorySeparatorChar;
         static const WCHAR AltDirectorySeparatorChar;
 public:
-        static BOOL IsExtended(SString & path);
-        static BOOL IsUNCExtended(SString & path);
         static BOOL ContainsDirectorySeparator(SString & path);
         static BOOL IsDirectorySeparator(WCHAR c);
-        static BOOL IsPathNotFullyQualified(SString & path);
-        static BOOL IsDevice(SString & path);
+        static BOOL IsPathNotFullyQualified(const SString & path);
 
         static HRESULT NormalizePath(SString& path);
 
 #ifdef HOST_WINDOWS
+        static BOOL IsExtended(const SString & path);
+        static BOOL IsUNCExtended(const SString & path);
+        static BOOL IsDevice(const SString & path);
         static void NormalizeDirectorySeparators(SString& path);
 #endif
 };
@@ -786,15 +786,14 @@ void LongFile::NormalizeDirectorySeparators(SString& path)
     }
 }
 
-BOOL LongFile::IsExtended(SString & path)
+BOOL LongFile::IsExtended(const SString & path)
 {
-    return path.BeginsWith(ExtendedPrefix);
+    return path.BeginsWith(SL(ExtendedPrefix));
 }
 
-BOOL LongFile::IsUNCExtended(SString & path)
+BOOL LongFile::IsUNCExtended(const SString & path)
 {
-
-    return path.BeginsWith(UNCExtendedPathPrefix);
+    return path.BeginsWith(SL(UNCExtendedPathPrefix));
 }
 
 // Relative here means it could be relative to current directory on the relevant drive
@@ -805,7 +804,7 @@ BOOL LongFile::IsUNCExtended(SString & path)
 // Handles paths that use the alternate directory separator.  It is a frequent mistake to
 // assume that rooted paths (Path.IsPathRooted) are not relative.  This isn't the case.
 
-BOOL LongFile::IsPathNotFullyQualified(SString & path)
+BOOL LongFile::IsPathNotFullyQualified(const SString & path)
 {
     if (path.GetCount() < 2)
     {
@@ -822,9 +821,9 @@ BOOL LongFile::IsPathNotFullyQualified(SString & path)
             && IsDirectorySeparator(path[2]));
 }
 
-BOOL LongFile::IsDevice(SString & path)
+BOOL LongFile::IsDevice(const SString & path)
 {
-    return path.BeginsWith(DevicePathPrefix);
+    return path.BeginsWith(SL(DevicePathPrefix));
 }
 
 // This function will normalize paths if the path length exceeds MAX_PATH
@@ -848,7 +847,7 @@ HRESULT LongFile::NormalizePath(SString & path)
     SString prefix(ExtendedPrefix);
     prefixLen = prefix.GetCount();
 
-    if (path.BeginsWith(UNCPathPrefix))
+    if (path.BeginsWith(SL(UNCPathPrefix)))
     {
         prefix.Set(UNCExtendedPathPrefix);
         //In this case if path is \\server the extended syntax should be like  \\?\UNC\server
@@ -898,9 +897,8 @@ HRESULT LongFile::NormalizePath(SString & path)
        SString fullpath(SString::Literal,buffer + prefixLen);
 
     //Check if the resolved path is a UNC. By default we assume relative path to resolve to disk
-    if (fullpath.BeginsWith(UNCPathPrefix) && prefixLen != prefix.GetCount() - (COUNT_T)wcslen(UNCPATHPREFIX))
+    if (fullpath.BeginsWith(SL(UNCPathPrefix)) && prefixLen != prefix.GetCount() - (COUNT_T)wcslen(UNCPATHPREFIX))
     {
-
         //Remove the leading '\\' from the UNC path to be replaced with UNCExtendedPathPrefix
         fullpath.Replace(fullpath.Begin(), (COUNT_T)wcslen(UNCPATHPREFIX), UNCExtendedPathPrefix);
         path.CloseBuffer();
@@ -918,26 +916,11 @@ HRESULT LongFile::NormalizePath(SString & path)
     return S_OK;
 }
 #else
-BOOL LongFile::IsExtended(SString & path)
-{
-    return FALSE;
-}
-
-BOOL LongFile::IsUNCExtended(SString & path)
-{
-    return FALSE;
-}
-
-BOOL LongFile::IsPathNotFullyQualified(SString & path)
+BOOL LongFile::IsPathNotFullyQualified(const SString & path)
 {
     return TRUE;
 }
 
-BOOL LongFile::IsDevice(SString & path)
-{
-    return FALSE;
-}
-
 //Don't need to do anything For XPlat
 HRESULT LongFile::NormalizePath(SString & path)
 {

diff --git a/src/native/corehost/deps_format.cpp b/src/native/corehost/deps_format.cpp
index fdfc186..4f933e4 100644 (file)
--- a/src/native/corehost/deps_format.cpp
+++ b/src/native/corehost/deps_format.cpp
@@ -96,8 +96,6 @@ void deps_json_t::reconcile_libraries_with_targets(
                 entry.asset = asset;
                 entry.asset.name = asset_name;
 
-                m_deps_entries[i].push_back(entry);
-
                 if (trace::is_enabled())
                 {
                     trace::info(_X("Parsed %s deps entry %d for asset name: %s from %s: %s, library version: %s, relpath: %s, assemblyVersion %s, fileVersion %s"),
@@ -111,6 +109,8 @@ void deps_json_t::reconcile_libraries_with_targets(
                         entry.asset.assembly_version.as_str().c_str(),
                         entry.asset.file_version.as_str().c_str());
                 }
+
+                m_deps_entries[i].push_back(std::move(entry));
             }
         }
     }
@@ -307,7 +307,7 @@ bool deps_json_t::process_targets(const json_parser_t::value_t& json, const pal:
                         package.name.GetString());
                 }
 
-                asset_files.push_back(asset);
+                asset_files.push_back(std::move(asset));
             }
         }
     }
@@ -413,7 +413,9 @@ bool deps_json_t::load_self_contained(const pal::string_t& deps_path, const json
 
 bool deps_json_t::has_package(const pal::string_t& name, const pal::string_t& ver) const
 {
-    pal::string_t pv = name;
+    pal::string_t pv;
+    pv.reserve(name.length() + ver.length() + 1);
+    pv.assign(name);
     pv.push_back(_X('/'));
     pv.append(ver);
 

diff --git a/src/native/corehost/hostpolicy/deps_resolver.cpp b/src/native/corehost/hostpolicy/deps_resolver.cpp
index aae9e87..8a0afa3 100644 (file)
--- a/src/native/corehost/hostpolicy/deps_resolver.cpp
+++ b/src/native/corehost/hostpolicy/deps_resolver.cpp
@@ -90,21 +90,22 @@ namespace
   // "asset_name" be part of the "items" paths.
   //
 void deps_resolver_t::add_tpa_asset(
-    const deps_resolved_asset_t& resolved_asset,
+    const deps_asset_t& asset,
+    const pal::string_t& resolved_path,
     name_to_resolved_asset_map_t* items)
 {
-    name_to_resolved_asset_map_t::iterator existing = items->find(resolved_asset.asset.name);
+    name_to_resolved_asset_map_t::iterator existing = items->find(asset.name);
     if (existing == items->end())
     {
         if (trace::is_enabled())
         {
             trace::verbose(_X("Adding tpa entry: %s, AssemblyVersion: %s, FileVersion: %s"),
-                resolved_asset.resolved_path.c_str(),
-                resolved_asset.asset.assembly_version.as_str().c_str(),
-                resolved_asset.asset.file_version.as_str().c_str());
+                resolved_path.c_str(),
+                asset.assembly_version.as_str().c_str(),
+                asset.file_version.as_str().c_str());
         }
 
-        items->emplace(resolved_asset.asset.name, resolved_asset);
+        items->emplace(asset.name, deps_resolved_asset_t(asset, resolved_path));
     }
 }
 
@@ -171,8 +172,7 @@ void deps_resolver_t::get_dir_assemblies(
                 file_path.c_str());
 
             deps_asset_t asset(file_name, file, empty, empty);
-            deps_resolved_asset_t resolved_asset(asset, file_path);
-            add_tpa_asset(resolved_asset, items);
+            add_tpa_asset(asset, file_path, items);
         }
     }
 }
@@ -464,8 +464,7 @@ bool deps_resolver_t::resolve_tpa_list(
                 // The runtime directly probes the bundle-manifest using a host-callback.
                 if (!found_in_bundle)
                 {
-                    deps_resolved_asset_t resolved_asset(entry.asset, resolved_path);
-                    add_tpa_asset(resolved_asset, &items);
+                    add_tpa_asset(entry.asset, resolved_path, &items);
                 }
 
                 return true;
@@ -511,8 +510,7 @@ bool deps_resolver_t::resolve_tpa_list(
                         if (!found_in_bundle)
                         {
                             deps_asset_t asset(entry.asset.name, entry.asset.relative_path, entry.asset.assembly_version, entry.asset.file_version);
-                            deps_resolved_asset_t resolved_asset(asset, resolved_path);
-                            add_tpa_asset(resolved_asset, &items);
+                            add_tpa_asset(asset, resolved_path, &items);
                         }
                     }
                 }
@@ -544,8 +542,7 @@ bool deps_resolver_t::resolve_tpa_list(
             bundle::runner_t::app()->probe(managed_app_name) == nullptr)
         {
             deps_asset_t asset(get_filename_without_ext(m_managed_app), managed_app_name, version_t(), version_t());
-            deps_resolved_asset_t resolved_asset(asset, m_managed_app);
-            add_tpa_asset(resolved_asset, &items);
+            add_tpa_asset(asset, m_managed_app, &items);
         }
 
         // Add the app's entries

diff --git a/src/native/corehost/hostpolicy/deps_resolver.h b/src/native/corehost/hostpolicy/deps_resolver.h
index 082cd5c..0c90820 100644 (file)
--- a/src/native/corehost/hostpolicy/deps_resolver.h
+++ b/src/native/corehost/hostpolicy/deps_resolver.h
@@ -254,7 +254,8 @@ private:
     pal::string_t m_app_dir;
 
     void add_tpa_asset(
-        const deps_resolved_asset_t& asset,
+        const deps_asset_t& asset,
+        const pal::string_t& resolved_path,
         name_to_resolved_asset_map_t* items);
 
     // Mode in which the host is being run. This can dictate how dependencies should be discovered.