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

Move handle store and remaining crsts from BaseDomain to AppDomain #107208

Merged
merged 4 commits into from
Sep 3, 2024

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Aug 30, 2024

  • The handle store was always the global handle store
    • Make all the Create*Handle functions go through the AppDomain
  • m_crstLoaderAllocatorReferences is only used for collectible loader allocators - so only assembly loader allocators (which correspond to AppDomain), not the global loader allocator (which corresponds to SystemDomain)
  • Remove unnecessary BaseDomain on PinnedHeapHandleTable

cc @jkotas @AaronRobinsonMSFT

Copy link
Contributor

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

CrstExplicitInit m_crstLoaderAllocatorReferences;

IGCHandleStore* m_handleStore;

// The pinned heap handle table.
PinnedHeapHandleTable *m_pPinnedHeapHandleTable;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only remaining thing on BaseDomain. Currently, AppDomain and SystemDomain do actually create/use two separate instances. I don't think they need to though?

For SystemDomain it is used via the global loader allocator:

OBJECTREF* pRef = GetDomain()->AllocateObjRefPtrsInLargeTable(1);

GetDomain()->AllocateObjRefPtrsInLargeTable(cSlots, pStaticsInfo, pMTToFillWithStaticBoxes, isClassInitedByUpdatingStaticPointer);

For AppDomain, it appears to just be for EnC:

*pOR = pDomain->AllocateStaticFieldObjRefPtrs(1);

*pOR = pDomain->AllocateStaticFieldObjRefPtrs(1);

@elinor-fung elinor-fung merged commit 5026865 into dotnet:main Sep 3, 2024
88 of 90 checks passed
@elinor-fung elinor-fung deleted the base-domain-handle-store branch September 3, 2024 21:39
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Sep 6, 2024
dotnet#107208)

- The handle store was always the global handle store
  - Make all the `Create*Handle` functions go through the `AppDomain`
- `m_crstLoaderAllocatorReferences` is only used for collectible loader allocators - so only assembly loader allocators (which correspond to `AppDomain`), not the global loader allocator (which corresponds to `SystemDomain`)
- Remove unnecessary `BaseDomain` on `PinnedHeapHandleTable`
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
dotnet#107208)

- The handle store was always the global handle store
  - Make all the `Create*Handle` functions go through the `AppDomain`
- `m_crstLoaderAllocatorReferences` is only used for collectible loader allocators - so only assembly loader allocators (which correspond to `AppDomain`), not the global loader allocator (which corresponds to `SystemDomain`)
- Remove unnecessary `BaseDomain` on `PinnedHeapHandleTable`
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
dotnet#107208)

- The handle store was always the global handle store
  - Make all the `Create*Handle` functions go through the `AppDomain`
- `m_crstLoaderAllocatorReferences` is only used for collectible loader allocators - so only assembly loader allocators (which correspond to `AppDomain`), not the global loader allocator (which corresponds to `SystemDomain`)
- Remove unnecessary `BaseDomain` on `PinnedHeapHandleTable`
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants