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.

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

Allocate RuntimeType objects on Frozen Object Heap #75573

Merged
merged 46 commits into from
Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
9c15c1f
Allocate Type objects on Frozen Heap
EgorBo Sep 13, 2022
232033b
Mitigate perf regressions
EgorBo Sep 13, 2022
f63c941
clean up
EgorBo Sep 13, 2022
841e9c7
fix build issue?
EgorBo Sep 14, 2022
f168d13
Fix eeGetCPString for string literals in JitDisasm
EgorBo Sep 14, 2022
2f50121
Merge branch 'main' of github.com:dotnet/runtime into foh-type-objects
EgorBo Sep 14, 2022
3e6d3ba
clean up
EgorBo Sep 14, 2022
c939027
Merge branch 'main' of github.com:dotnet/runtime into foh-type-objects
EgorBo Sep 14, 2022
c8512a9
Update src/coreclr/jit/compiler.h
EgorBo Sep 17, 2022
bad7cc4
Merge branch 'main' of github.com:dotnet/runtime into foh-type-objects
EgorBo Sep 17, 2022
9d8526e
Address some of the feedback [WIP]
EgorBo Sep 17, 2022
18e1cd4
Address feedback
EgorBo Sep 17, 2022
a7fea6d
Fix compilation
EgorBo Sep 17, 2022
f2afedc
Introduce `objectToString` JIT-EE API
EgorBo Sep 17, 2022
6a8cedf
Update src/coreclr/vm/methodtable.cpp
EgorBo Sep 17, 2022
677f94b
check for unloadability in GetPinnedManagedClassObjectIfExists
EgorBo Sep 17, 2022
15bcad1
Merge branch 'foh-type-objects' of github.com:EgorBo/runtime-1 into f…
EgorBo Sep 17, 2022
81bacbb
Since I still have to check CanUnload I guess I don't need the bit test
EgorBo Sep 17, 2022
b5b7c98
Address feedback
EgorBo Sep 17, 2022
608e6c9
Add more temp debug checks for ci
EgorBo Sep 17, 2022
be063b8
Check if it reproduces because of jit
EgorBo Sep 18, 2022
59af88b
Check if it reproduces because of jit
EgorBo Sep 18, 2022
a8e0786
Ah, facepalm
EgorBo Sep 18, 2022
1f9a348
Clean up
EgorBo Sep 18, 2022
2fd4384
Address Jan's feedback
EgorBo Sep 18, 2022
8f2a767
Oops, a typo
EgorBo Sep 18, 2022
b4b7128
Share logic to extract OBJECTREF from m_hExposedClassObject
EgorBo Sep 18, 2022
a2ca5b1
Merge branch 'main' of github.com:dotnet/runtime into foh-type-objects
EgorBo Sep 19, 2022
fd099a2
Produce better asm code for JIT_GetRuntimeType
EgorBo Sep 19, 2022
64e7171
Merge branch 'main' of github.com:dotnet/runtime into foh-type-objects
EgorBo Sep 20, 2022
ac405a1
Merge branch 'main' of github.com:dotnet/runtime into foh-type-objects
EgorBo Sep 20, 2022
7ab7bd1
Address feedback
EgorBo Sep 20, 2022
895e9f5
Update src/coreclr/vm/jitinterface.cpp
EgorBo Sep 20, 2022
bcd47b1
Address feedback
EgorBo Sep 20, 2022
c217579
Update src/coreclr/vm/methodtable.cpp
EgorBo Sep 21, 2022
253747e
Address feedback: the bit is always there so we can do ` - 1`
EgorBo Sep 21, 2022
ca6c69a
Address feedback
EgorBo Sep 21, 2022
93ef525
Update src/coreclr/vm/methodtable.cpp
jkotas Sep 21, 2022
7394e1e
Update src/coreclr/jit/ee_il_dll.cpp
EgorBo Sep 21, 2022
f6055b9
remove null-termination from api
EgorBo Sep 21, 2022
57b1477
address feedback
EgorBo Sep 21, 2022
e9a396f
Address feedback
EgorBo Sep 21, 2022
4fbb894
Pass utf8 string in objectToString
EgorBo Sep 21, 2022
7476a68
remove char16_t left over
EgorBo Sep 21, 2022
bc2292b
fix compilation warning
EgorBo Sep 22, 2022
7bf7e0e
Update gentree.cpp
EgorBo Sep 22, 2022
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
5 changes: 0 additions & 5 deletions src/coreclr/inc/clrconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,6 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_CodeHeapReserveForJumpStubs, W("CodeHeapReserv
RETAIL_CONFIG_DWORD_INFO(INTERNAL_NGenReserveForJumpStubs, W("NGenReserveForJumpStubs"), 0, "Percentage of ngen image size to reserve for jump stubs")
RETAIL_CONFIG_DWORD_INFO(INTERNAL_BreakOnOutOfMemoryWithinRange, W("BreakOnOutOfMemoryWithinRange"), 0, "Break before out of memory within range exception is thrown")

///
/// Frozen segments (aka Frozen Object Heap)
///
RETAIL_CONFIG_DWORD_INFO(INTERNAL_UseFrozenObjectHeap, W("UseFrozenObjectHeap"), 1, "Use frozen object heap for certain types of objects (e.g. string literals) as an optimization.")

///
/// Log
///
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -2265,6 +2265,13 @@ class ICorStaticInfo
int bufferSize /* IN */
) = 0;

// Calls ToString() for given pinned/frozen object handle
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
virtual int objectToString (
void* handle, /* IN */
char16_t* buffer, /* OUT */
int bufferSize /* IN */
) = 0;

/**********************************************************************************/
//
// ICorClassInfo
Expand Down Expand Up @@ -2475,6 +2482,10 @@ class ICorStaticInfo
CORINFO_CLASS_HANDLE cls
) = 0;

virtual void* getRuntimeTypePointer(
CORINFO_CLASS_HANDLE cls
) = 0;

virtual bool getReadyToRunHelper(
CORINFO_RESOLVED_TOKEN * pResolvedToken,
CORINFO_LOOKUP_KIND * pGenericLookupKind,
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/inc/crsttypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ enum CrstType
CrstVSDIndirectionCellLock = 114,
CrstWrapperTemplate = 115,
CrstFrozenObjectHeap = 116,
kNumberOfCrstTypes = 117
CrstMethodTableExposedObject = 117,
kNumberOfCrstTypes = 118
};

#endif // __CRST_TYPES_INCLUDED
Expand Down Expand Up @@ -259,7 +260,8 @@ int g_rgCrstLevelMap[] =
3, // CrstUnwindInfoTableLock
4, // CrstVSDIndirectionCellLock
3, // CrstWrapperTemplate
0, // CrstFrozenObjectHeap
-1, // CrstFrozenObjectHeap
-1, // CrstMethodTableExposedObject
};

// An array mapping CrstType to a stringized name.
Expand Down Expand Up @@ -381,7 +383,8 @@ LPCSTR g_rgCrstNameMap[] =
"CrstUnwindInfoTableLock",
"CrstVSDIndirectionCellLock",
"CrstWrapperTemplate",
"CrstFrozenObjectHeap"
"CrstFrozenObjectHeap",
"CrstMethodTableExposedObject"
};

// Define a special level constant for unordered locks.
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ int getStringLiteral(
char16_t* buffer,
int bufferSize) override;

int objectToString(
void* handle,
char16_t* buffer,
int bufferSize) override;

CorInfoType asCorInfoType(
CORINFO_CLASS_HANDLE cls) override;

Expand Down Expand Up @@ -280,6 +285,9 @@ CorInfoHelpFunc getBoxHelper(
CorInfoHelpFunc getUnBoxHelper(
CORINFO_CLASS_HANDLE cls) override;

void* getRuntimeTypePointer(
CORINFO_CLASS_HANDLE cls) override;

bool getReadyToRunHelper(
CORINFO_RESOLVED_TOKEN* pResolvedToken,
CORINFO_LOOKUP_KIND* pGenericLookupKind,
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* eb8352bd-0a13-4b5b-badb-58f9ecc40c44 */
0xeb8352bd,
0x0a13,
0x4b5b,
{0xba, 0xdb, 0x58, 0xf9, 0xec, 0xc4, 0x0c, 0x44}
constexpr GUID JITEEVersionIdentifier = { /* 11b4ea58-c400-4c3d-995e-4e2f0676f6e8 */
0x11b4ea58,
0xc400,
0x4c3d,
{0x99, 0x5e, 0x4e, 0x2f, 0x6, 0x76, 0xf6, 0xe8}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/ICorJitInfo_API_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ DEF_CLR_API(getTokenTypeAsHandle)
DEF_CLR_API(isValidToken)
DEF_CLR_API(isValidStringRef)
DEF_CLR_API(getStringLiteral)
DEF_CLR_API(objectToString)
DEF_CLR_API(asCorInfoType)
DEF_CLR_API(getClassName)
DEF_CLR_API(getClassNameFromMetadata)
Expand Down Expand Up @@ -70,6 +71,7 @@ DEF_CLR_API(getSharedCCtorHelper)
DEF_CLR_API(getTypeForBox)
DEF_CLR_API(getBoxHelper)
DEF_CLR_API(getUnBoxHelper)
DEF_CLR_API(getRuntimeTypePointer)
DEF_CLR_API(getReadyToRunHelper)
DEF_CLR_API(getReadyToRunDelegateCtorHelper)
DEF_CLR_API(getHelperName)
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/jit/ICorJitInfo_API_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,17 @@ int WrapICorJitInfo::getStringLiteral(
return temp;
}

int WrapICorJitInfo::objectToString(
void* handle,
char16_t* buffer,
int bufferSize)
{
API_ENTER(objectToString);
int temp = wrapHnd->objectToString(handle, buffer, bufferSize);
API_LEAVE(objectToString);
return temp;
}

CorInfoType WrapICorJitInfo::asCorInfoType(
CORINFO_CLASS_HANDLE cls)
{
Expand Down Expand Up @@ -652,6 +663,15 @@ CorInfoHelpFunc WrapICorJitInfo::getUnBoxHelper(
return temp;
}

void* WrapICorJitInfo::getRuntimeTypePointer(
CORINFO_CLASS_HANDLE cls)
{
API_ENTER(getRuntimeTypePointer);
void* temp = wrapHnd->getRuntimeTypePointer(cls);
API_LEAVE(getRuntimeTypePointer);
return temp;
}

bool WrapICorJitInfo::getReadyToRunHelper(
CORINFO_RESOLVED_TOKEN* pResolvedToken,
CORINFO_LOOKUP_KIND* pGenericLookupKind,
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2149,7 +2149,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion)
break;
case O1K_LCLVAR:
assert((lvaGetDesc(assertion->op1.lcl.lclNum)->lvType != TYP_REF) ||
(assertion->op2.u1.iconVal == 0) || doesMethodHaveFrozenString());
(assertion->op2.u1.iconVal == 0) || doesMethodHaveFrozenObjects());
break;
case O1K_VALUE_NUMBER:
assert((vnStore->TypeOfVN(assertion->op1.vn) != TYP_REF) || (assertion->op2.u1.iconVal == 0));
Expand Down Expand Up @@ -3423,7 +3423,7 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion,

// Make sure we don't retype const gc handles to TYP_I_IMPL
// Although, it's possible for e.g. GTF_ICON_STATIC_HDL
if (!newTree->IsIntegralConst(0) && newTree->IsIconHandle(GTF_ICON_STR_HDL))
if (!newTree->IsIntegralConst(0) && newTree->IsIconHandle(GTF_ICON_OBJ_HDL))
{
if (tree->TypeIs(TYP_BYREF))
{
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9417,6 +9417,11 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
chars += printf("[ICON_STR_HDL]");
break;

case GTF_ICON_OBJ_HDL:

chars += printf("[ICON_OBJ_HDL]");
break;

case GTF_ICON_CONST_PTR:

chars += printf("[ICON_CONST_PTR]");
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6815,7 +6815,7 @@ class Compiler
#define OMF_HAS_EXPRUNTIMELOOKUP 0x00000080 // Method contains a runtime lookup to an expandable dictionary.
#define OMF_HAS_PATCHPOINT 0x00000100 // Method contains patchpoints
#define OMF_NEEDS_GCPOLLS 0x00000200 // Method needs GC polls
#define OMF_HAS_FROZEN_STRING 0x00000400 // Method has a frozen string (REF constant int), currently only on NativeAOT.
#define OMF_HAS_FROZEN_OBJECTS 0x00000400 // Method has frozen objects (REF constant int)
#define OMF_HAS_PARTIAL_COMPILATION_PATCHPOINT 0x00000800 // Method contains partial compilation patchpoints
#define OMF_HAS_TAILCALL_SUCCESSOR 0x00001000 // Method has potential tail call in a non BBJ_RETURN block
#define OMF_HAS_MDNEWARRAY 0x00002000 // Method contains 'new' of an MD array
Expand All @@ -6840,14 +6840,14 @@ class Compiler

void addFatPointerCandidate(GenTreeCall* call);

bool doesMethodHaveFrozenString() const
bool doesMethodHaveFrozenObjects() const
{
return (optMethodFlags & OMF_HAS_FROZEN_STRING) != 0;
return (optMethodFlags & OMF_HAS_FROZEN_OBJECTS) != 0;
}

void setMethodHasFrozenString()
void setMethodHasFrozenObjects()
{
optMethodFlags |= OMF_HAS_FROZEN_STRING;
optMethodFlags |= OMF_HAS_FROZEN_OBJECTS;
}

bool doesMethodHaveGuardedDevirtualization() const
Expand Down Expand Up @@ -7807,7 +7807,7 @@ class Compiler
const char* eeGetFieldName(CORINFO_FIELD_HANDLE fieldHnd, const char** classNamePtr = nullptr);

#if defined(DEBUG)
const WCHAR* eeGetCPString(size_t stringHandle);
void eePrintFrozenObjectDescription(const char* prefix, size_t handle);
unsigned eeTryGetClassSize(CORINFO_CLASS_HANDLE clsHnd);
const char16_t* eeGetShortClassName(CORINFO_CLASS_HANDLE clsHnd);
#endif
Expand Down
49 changes: 22 additions & 27 deletions src/coreclr/jit/ee_il_dll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1615,45 +1615,40 @@ const char16_t* Compiler::eeGetShortClassName(CORINFO_CLASS_HANDLE clsHnd)
return param.classNameWidePtr;
}

const WCHAR* Compiler::eeGetCPString(size_t strHandle)
void Compiler::eePrintFrozenObjectDescription(const char* prefix, size_t handle)
{
#ifdef HOST_UNIX
return nullptr;
#else
char buff[512 + sizeof(CORINFO_String)];

// make this bulletproof, so it works even if we are wrong.
if (ReadProcessMemory(GetCurrentProcess(), (void*)strHandle, buff, 4, nullptr) == 0)
{
return (nullptr);
}

CORINFO_String* asString = nullptr;
if (impGetStringClass() == *((CORINFO_CLASS_HANDLE*)strHandle))
const int maxStrSize = 64;
char16_t str[maxStrSize];
int realLength = this->info.compCompHnd->objectToString((void*)handle, str, maxStrSize);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
if (realLength >= maxStrSize)
{
// strHandle is a frozen string
// We assume strHandle is never an "interior" pointer in a frozen string
// (jit is not expected to perform such foldings)
asString = (CORINFO_String*)strHandle;
str[maxStrSize - 4] = L'.';
str[maxStrSize - 3] = L'.';
str[maxStrSize - 2] = L'.';
str[maxStrSize - 1] = 0;
}
else
{
// strHandle is a pinned handle to a string object
asString = *((CORINFO_String**)strHandle);
str[realLength] = 0;
}

if (ReadProcessMemory(GetCurrentProcess(), asString, buff, sizeof(buff), nullptr) == 0)
for (size_t i = 0; i < min(maxStrSize, realLength); i++)
{
return (nullptr);
// Escape \n and \r symbols
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
if (str[i] == L'\n' || str[i] == L'\r')
{
str[i] = L' ';
}
}

if (asString->stringLen >= 255 || asString->chars[asString->stringLen] != 0)
if (realLength >= 0)
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
return nullptr;
printf("%s '%S'\n", prefix, str);
Copy link
Member

@jkotas jkotas Sep 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to return the string from the JIT/EE interface API as utf8, similar to other places that return names.

The problem with depending on %S to format char16_t strings is that depends on the PAL emulating Windows CRT behavior on Unix. We have been working towards getting rid of the Windows PAL emulator, and adding more uses of %S goes against that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkotas I was thinking of leaving it as is just for future JIT optimizations e.g. enable unrolling of Equals/StartsWith for static readonly strings, folding some .ToString() operations etc and for that I'll need raw content

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to do that, it should be a new API that just returns the raw string content.

I do not think it is a good idea to be overloading APIs to do both pretty printing and real functionality.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with UTF8 char*
The only problem that it now prints garbage for non latin1 string literals for JitDisasm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same issue exists in number of other places (e.g. for method names). I do not think that it is a problem. Do you agree?

I think we should standardize on utf8 output, and tell everybody who is not happy about the garbage characters from jit tracing to switch their console encoding to utf8. I have my console encoding set to utf8 on all my Windows systems for a while, so I almost forgot that this problem exists.

}
else
{
printf("%s 'frozen object handle'\n", prefix, str);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}

return (WCHAR*)(asString->chars);
#endif // HOST_UNIX
}
#else // DEBUG
void jitprintf(const char* fmt, ...)
Expand Down
39 changes: 6 additions & 33 deletions src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4129,42 +4129,15 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag

const char* str = nullptr;
if (flag == GTF_ICON_STR_HDL)
{
str = "string handle";
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}
else if (flag == GTF_ICON_OBJ_HDL)
{
#ifdef DEBUG
const WCHAR* wstr = emitComp->eeGetCPString(handle);
// NOTE: eGetCPString always returns nullptr on Linux/ARM
if (wstr == nullptr)
{
str = "string handle";
}
else
{
const size_t actualLen = wcslen(wstr);
const size_t maxLength = 63;
const size_t newLen = min(maxLength, actualLen);

// +1 for null terminator
WCHAR buf[maxLength + 1] = {0};
wcsncpy(buf, wstr, newLen);
for (size_t i = 0; i < newLen; i++)
{
// Escape \n and \r symbols
if (buf[i] == L'\n' || buf[i] == L'\r')
{
buf[i] = L' ';
}
}
if (actualLen > maxLength)
{
// Append "..." for long strings
buf[maxLength - 3] = L'.';
buf[maxLength - 2] = L'.';
buf[maxLength - 1] = L'.';
}
printf("%s \"%S\"", commentPrefix, buf);
}
emitComp->eePrintFrozenObjectDescription(commentPrefix, handle);
#else
str = "string handle";
str = "frozen object handle";
#endif
}
else if (flag == GTF_ICON_CLASS_HDL)
Expand Down
12 changes: 0 additions & 12 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8756,18 +8756,6 @@ void emitter::emitDispAddrMode(instrDesc* id, bool noDetail)

printf("]");

// pretty print string if it looks like one
#ifdef DEBUG
if ((id->idGCref() == GCT_GCREF) && (id->idIns() == INS_mov) && (id->idAddr()->iiaAddrMode.amBaseReg == REG_NA))
{
const WCHAR* str = emitComp->eeGetCPString(disp);
if (str != nullptr)
{
printf(" '%S'", str);
}
}
#endif

if (jdsc && !noDetail)
{
unsigned cnt = (jdsc->dsSize - 1) / TARGET_POINTER_SIZE;
Expand Down
Loading