Skip to content
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

Jump to bottom

Cache the path on Module for easier diagnostics #106103

Merged
merged 5 commits into from
Aug 9, 2024
Merged

Cache the path on Module for easier diagnostics #106103

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
15226d3
Make PEImage path const
elinor-fung Aug 6, 2024
64ec994
Cache path on Module
elinor-fung Aug 6, 2024
fd247d4
Apply suggestions from code review
elinor-fung Aug 8, 2024
b2062d4
Make Module::GetPath assert that it matches the cached path
elinor-fung Aug 9, 2024
c508c1e
Update src/coreclr/vm/ceeload.h
elinor-fung Aug 9, 2024
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
6 changes: 6 additions & 0 deletions src/coreclr/inc/sstring.inl
View file
Open in desktop
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
View file
Open in desktop
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
12 changes: 7 additions & 5 deletions src/coreclr/vm/ceeload.h
View file
Open in desktop
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
lambdageek marked this conversation as resolved.
Show resolved Hide resolved

PTR_PEAssembly m_pPEAssembly;
PTR_VOID m_baseAddress; // Cached base address for easier diagnostics
Expand Down Expand Up @@ -1384,7 +1381,12 @@ 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;
_ASSERTE(SString{m_path}.Equals(m_pPEAssembly->GetPath()));
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading