-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Weak interior handles #100446
Weak interior handles #100446
Conversation
…sistent between collectible and non-collectible types
Tagging subscribers to this area: @mangod9 |
@Maoni0 I'd like it if you could take a look at this sometime. I've not managed to test it ... but this is the sort of thing I want to do |
src/coreclr/vm/loaderallocator.cpp
Outdated
// | ||
// We work around these details by the following means | ||
// 1. We use a LOADERHANDLE to keep the object alive until the LoaderAllocator is freed. | ||
// 2. We hold the crstLoaderAllocatorLock, and double check to wnsure that the data is ready to be filled in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 2. We hold the crstLoaderAllocatorLock, and double check to wnsure that the data is ready to be filled in | |
// 2. We hold the crstLoaderAllocatorLock, and double check to ensure that the data is ready to be filled in |
I presume the extra info for these handles must always point to the middle of the primary target of this handle (it's not obvious from the caller I'm seeing in loaderallocator.cpp). we should add some verification code that this is the case. I thought I'd mention we do have a `GCHeap::GetContainingObject" that gets you from an interior pointer to its containing object, in case this is useful to you. otherwise the change looks ok to me. |
Don't forget to set the typehandle into the RuntimeType object
…ch point into the middle of the specified object
Ok, done.
Unfortunately my understanding is that api can only be called when the runtime is suspended due to race conditions, so I can't use this in any of the cases I'm working with. |
src/coreclr/gc/handletablescan.cpp
Outdated
// if we did then copy the value | ||
if (pUserData) | ||
{ | ||
TADDR pObjectInteriorPointer = **dac_cast<DPTR(DPTR(TADDR))>(pUserData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: None of the surrounding code is DACized so dac_cast is unnecessary complication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought that VerifyHeap worked in the DAC, but it turns out that it is an entirely parallel managed implementation these days. I'll pull that out if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a build break introduced by the dac_cast
macro. My guess is that the easiest way to fix it is to delete the use here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a potential follow up codegen improvement for collectible assemblies:
We can change getRuntimeTypePointer
to return InfoAccessType
in addition to the pointer itself and allow JIT to fetch the Type object by a simple dereference instead of calling the helper in collectible code.
cc @EgorBo
src/coreclr/vm/loaderallocator.cpp
Outdated
// | ||
// We work around these details by the following means | ||
// 1. We use a LOADERHANDLE to keep the object alive until the LoaderAllocator is freed. | ||
// 2. We hold the crstLoaderAllocatorLock, and double check to ensure that the data is ready to be filled in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 2. We hold the crstLoaderAllocatorLock, and double check to ensure that the data is ready to be filled in | |
// 2. We hold the m_crstLoaderAllocator lock, and double check to ensure that the data is ready to be filled in |
Nit
src/coreclr/vm/typedesc.h
Outdated
CONTRACTL_END; | ||
|
||
const RUNTIMETYPEHANDLE handle = m_hExposedClassObject; | ||
OBJECTREF retVal = ObjectToOBJECTREF((Object*)handle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OBJECTREF retVal = ObjectToOBJECTREF((Object*)handle); | |
OBJECTREF retVal = ObjectToOBJECTREF(handle); |
RUNTIMETYPEHANDLE can be defined as Object*
to save casts like this. It is actually Object* now.
- Implementation of weak interior handle - Use new weak interior handle to make managed class object storage consistent between collectible and non-collectible types
Add a new type of handle (HNDTYPE_WEAK_INTERIOR_POINTER) and a use of it to make storage of managed class objects consistent between collectible and non-collectible variants