Skip to content

Commit

Permalink
Cache the path on Module for easier diagnostics (#106103)
Browse files Browse the repository at this point in the history
- Make `PEImage` take a path in its constructor and make its member const
- Store the path when initializing `Module`

This will let us avoid having to expose PEAssembly/PEImage in data descriptors to the cDAC just to get the module path.
  • Loading branch information
elinor-fung committed Aug 9, 2024
1 parent 2ae5d7a commit 0953757
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 22 deletions.
6 changes: 6 additions & 0 deletions src/coreclr/inc/sstring.inl
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ inline SString::SString(const WCHAR *string)

Set(string);

_ASSERTE(IsRepresentation(REPRESENTATION_UNICODE));
SetNormalized();

SS_RETURN;
}

Expand All @@ -249,6 +252,9 @@ inline SString::SString(const WCHAR *string, COUNT_T count)

Set(string, count);

_ASSERTE(IsRepresentation(REPRESENTATION_UNICODE));
SetNormalized();

SS_RETURN;
}

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/ceeload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,8 @@ void Module::Initialize(AllocMemTracker *pamTracker, LPCWSTR szName)

m_loaderAllocator = GetAssembly()->GetLoaderAllocator();
m_pSimpleName = m_pPEAssembly->GetSimpleName();
m_path = m_pPEAssembly->GetPath().GetUnicode();
_ASSERTE(m_path != NULL);
m_baseAddress = m_pPEAssembly->HasLoadedPEImage() ? m_pPEAssembly->GetLoadedLayout()->GetBase() : NULL;
if (m_pPEAssembly->IsReflectionEmit())
m_dwTransientFlags |= IS_REFLECTION_EMIT;
Expand Down
13 changes: 8 additions & 5 deletions src/coreclr/vm/ceeload.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@
#include "ilinstrumentation.h"
#include "codeversion.h"

class Stub;
class MethodDesc;
class FieldDesc;
class Crst;
class ClassConverter;
class RefClassWriter;
class ReflectionModule;
class EEStringData;
Expand All @@ -56,11 +54,9 @@ class SigTypeContext;
class Assembly;
class BaseDomain;
class AppDomain;
class DomainModule;
class SystemDomain;
class Module;
class SString;
class Pending;
class MethodTable;
class DynamicMethodTable;
class TieredCompilationManager;
Expand Down Expand Up @@ -615,6 +611,7 @@ class Module : public ModuleBase

private:
PTR_CUTF8 m_pSimpleName; // Cached simple name for better performance and easier diagnostics
const WCHAR* m_path; // Cached path for easier diagnostics

PTR_PEAssembly m_pPEAssembly;
PTR_VOID m_baseAddress; // Cached base address for easier diagnostics
Expand Down Expand Up @@ -1384,7 +1381,13 @@ class Module : public ModuleBase
}

HRESULT GetScopeName(LPCUTF8 * pszName) { WRAPPER_NO_CONTRACT; return m_pPEAssembly->GetScopeName(pszName); }
const SString &GetPath() { WRAPPER_NO_CONTRACT; return m_pPEAssembly->GetPath(); }
const SString &GetPath()
{
WRAPPER_NO_CONTRACT;
// Validate the pointers are the same to ensure the lifetime of m_path is handled.
_ASSERTE(m_path == m_pPEAssembly->GetPath().GetUnicode());
return m_pPEAssembly->GetPath();
}

#ifdef LOGGING
LPCUTF8 GetDebugName() { WRAPPER_NO_CONTRACT; return m_pPEAssembly->GetDebugName(); }
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/vm/peimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,9 +615,8 @@ void PEImage::EnumMemoryRegions(CLRDataEnumMemoryFlags flags)

#endif // #ifdef DACCESS_COMPILE


PEImage::PEImage():
m_path(),
PEImage::PEImage(const WCHAR* path):
m_path{path},
m_pathHash(0),
m_refCount(1),
m_bInHashMap(FALSE),
Expand Down Expand Up @@ -788,7 +787,7 @@ PTR_PEImage PEImage::CreateFromByteArray(const BYTE* array, COUNT_T size)
}
CONTRACT_END;

PEImageHolder pImage(new PEImage());
PEImageHolder pImage(new PEImage(NULL /*path*/));
PTR_PEImageLayout pLayout = PEImageLayout::CreateFromByteArray(pImage, array, size);
_ASSERTE(!pLayout->IsMapped());

Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/vm/peimage.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class PEImage final
static void Startup();

~PEImage();
PEImage();
explicit PEImage(const WCHAR* path);

BOOL Equals(PEImage* pImage);

Expand All @@ -121,7 +121,7 @@ class PEImage final
MDInternalImportFlags flags = MDInternalImport_Default,
BundleFileLocation bundleFileLocation = BundleFileLocation::Invalid());

static PTR_PEImage FindByPath(LPCWSTR pPath, BOOL isInBundle = TRUE);
static PTR_PEImage FindByPath(LPCWSTR pPath, BOOL isInBundle);
void AddToHashMap();
#endif

Expand Down Expand Up @@ -206,7 +206,7 @@ class PEImage final
// Private routines
// ------------------------------------------------------------

void Init(LPCWSTR pPath, BundleFileLocation bundleFileLocation);
void Init(BundleFileLocation bundleFileLocation);

struct PEImageLocator
{
Expand Down Expand Up @@ -285,7 +285,7 @@ class PEImage final
// Instance fields
// ------------------------------------------------------------

SString m_path;
const SString m_path;
ULONG m_pathHash;
LONG m_refCount;

Expand Down
16 changes: 7 additions & 9 deletions src/coreclr/vm/peimage.inl
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ inline CHECK PEImage::CheckFormat()
CHECK_OK;
}

inline void PEImage::Init(LPCWSTR pPath, BundleFileLocation bundleFileLocation)
inline void PEImage::Init(BundleFileLocation bundleFileLocation)
{
CONTRACTL
{
Expand All @@ -286,8 +286,6 @@ inline void PEImage::Init(LPCWSTR pPath, BundleFileLocation bundleFileLocation)
}
CONTRACTL_END;

m_path.Set(pPath);
m_path.Normalize();
m_pathHash = m_path.HashCaseInsensitive();
m_bundleFileLocation = bundleFileLocation;
SetModuleFileNameHintForDAC();
Expand All @@ -296,7 +294,7 @@ inline void PEImage::Init(LPCWSTR pPath, BundleFileLocation bundleFileLocation)


/*static*/
inline PTR_PEImage PEImage::FindByPath(LPCWSTR pPath, BOOL isInBundle /* = TRUE */)
inline PTR_PEImage PEImage::FindByPath(LPCWSTR pPath, BOOL isInBundle)
{
CONTRACTL
{
Expand All @@ -316,13 +314,13 @@ inline PTR_PEImage PEImage::FindByPath(LPCWSTR pPath, BOOL isInBundle /* = TRUE
}

/* static */
inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags /* = MDInternalImport_Default */, BundleFileLocation bundleFileLocation)
inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags /* = MDInternalImport_Default */, BundleFileLocation bundleFileLocation /* = BundleFileLocation::Invalid() */)
{
BOOL forbidCache = (flags & MDInternalImport_NoCache);
if (forbidCache)
{
PEImageHolder pImage(new PEImage);
pImage->Init(pPath, bundleFileLocation);
PEImageHolder pImage(new PEImage{pPath});
pImage->Init(bundleFileLocation);
return dac_cast<PTR_PEImage>(pImage.Extract());
}

Expand All @@ -337,8 +335,8 @@ inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags
return NULL;
}

PEImageHolder pImage(new PEImage);
pImage->Init(pPath, bundleFileLocation);
PEImageHolder pImage(new PEImage{pPath});
pImage->Init(bundleFileLocation);

pImage->AddToHashMap();
return dac_cast<PTR_PEImage>(pImage.Extract());
Expand Down

0 comments on commit 0953757

Please sign in to comment.