Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[Local GC] Move handle creation to IGCHandleTable#10757

Merged
adityamandaleeka merged 4 commits intodotnet:masterfrom
adityamandaleeka:handle_table_local_gc_init
Apr 7, 2017
Merged

[Local GC] Move handle creation to IGCHandleTable#10757
adityamandaleeka merged 4 commits intodotnet:masterfrom
adityamandaleeka:handle_table_local_gc_init

Conversation

@adityamandaleeka
Copy link
Member

This change adds handle creation functions to the IGCHandleTable interface, and modifies the VM to use the interface for handle creation rather than calling functions in objecthandle.h directly.

Rather than adding a separate function for each Create__<type>__Handle function (of which there are many) to the interface, I consolidated them to a few major categories and added the Create__<type>__Handle functions to a header on the VM side that will call the appropriate thing on the interface. This minimizes changes to VM source while also not adding clutter to the interface.

I've also moved the handle-related code that was in gcheaputilities to its own file, since there is now a lot more code there.

Most of the handle creation functions are pretty straightforward, but there are a couple of interesting cases:

  • I removed CreateDuplicateHandle entirely since it's only used in one place in the VM and instead modified the caller to achieve the same result with the other functionality we already have on the interface.
  • Rather than adding an interface method specific to WinRT weak handles, I made CreateHandleWithExtraInfo which takes a void* that can be anything the user wants. CreateWinRTWeakHandle is now a convenience function that lives in the VM side and calls CreateHandleWithExtraInfo with the IWeakReference as the extra info.

@Maoni0 @jkotas @swgillespie @sergiy-k


virtual void* GetHandleTableForHandle(OBJECTHANDLE handle);

virtual OBJECTHANDLE CreateHandleOfType(void* table, Object* object, int type);

Choose a reason for hiding this comment

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

Should the definition of OBJECTHANDLE be on the interface? I see it's defined in vars.hpp and gcenv.base.h today.

@@ -1323,7 +1326,9 @@ class BaseDomain
OBJECTHANDLE CreateDependentHandle(OBJECTREF primary, OBJECTREF secondary)
{
WRAPPER_NO_CONTRACT;

Choose a reason for hiding this comment

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

This isn't a wrapper anymore - can we use ::CreateDepehdentHandle's contract here?

@adityamandaleeka adityamandaleeka force-pushed the handle_table_local_gc_init branch 2 times, most recently from fee973d to dd86429 Compare April 7, 2017 01:23
@adityamandaleeka
Copy link
Member Author

@dotnet-bot test Ubuntu arm Cross Release Build
(the failure is obviously not related to these changes)

return ::CreateTypedHandle(m_hHandleTableBucket->pTable[GetCurrentThreadHomeHeapNumber()], object, type);

IGCHandleTable *pHandleTable = GCHandleTableUtilities::GetGCHandleTable();
return pHandleTable->CreateHandleOfType(m_hHandleTableBucket->pTable[GetCurrentThreadHomeHeapNumber()], OBJECTREFToObject(object), type);
Copy link
Member

Choose a reason for hiding this comment

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

HandleTableBucket seems to be implementation detail of the HandleTable. Shouldn't it move to the GC side as well?

Also, it does not look right that I have to call to GC two times to create the handle: first to get thread home number, and then to actually create the handle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that will be addressed in a future PR. The home heap number will not be exposed anymore.

@jkotas
Copy link
Member

jkotas commented Apr 7, 2017

LGTM - as an intermediate point.

@adityamandaleeka
Copy link
Member Author

Thanks!

@adityamandaleeka adityamandaleeka force-pushed the handle_table_local_gc_init branch from dd86429 to 4475214 Compare April 7, 2017 22:02
@adityamandaleeka adityamandaleeka merged commit d276140 into dotnet:master Apr 7, 2017
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…table_local_gc_init

[Local GC] Move handle creation to IGCHandleTable

Commit migrated from dotnet/coreclr@d276140
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants