Split up loader heap implementations by davidwrighton · Pull Request #114246 · dotnet/runtime

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton · 2025-04-03T23:31:57Z

We have long had 3 rather different heaps embedded in one structure called a LoaderHeap. They have shared ... some infrastructure, but there is a mess of confusing flags and code paths which only work for one or the other type of heap.

This PR changes that to supporting the 3 different types of heaps with separate codebases, and keeps as much of the shared infrastructure as I could manage. Its, not what I would call a pretty separation, as there is a bit of a mess around the way the DAC apis can access the heaps, but I think this is an improvement. Notably, in a followon PR I'm intending to provide a new mechanism for the interleaved heap to work off of contents of a file instead of creating unique pages per stub block. (This should slighly improve the cache locality of .NET programs, and is a prerequisite for running on some heavily locked down platforms.) This PR prepares for that by making the Interleaved heap its own thing, so adding new behavior variants is a reviewable piece of work.

The only functional change that happens with this PR is the newly added capability of the interleaved heaps to support freeing memory. (It was always possible to free, but for some reason the re-allocation behavior was not actually supported.)

…_heap_implementations

…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

…ared anymore Add support for a free list that actually works in the stub heaps

dotnet-policy-service · 2025-04-03T23:32:43Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

…_heap_implementations

Copilot

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

src/coreclr/utilcode/CMakeLists.txt: Language not supported

jkotas · 2025-04-09T02:25:09Z

+ "\nset the following registry DWORD value:"
+ "\n"
+ "\n HKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"


Suggested change
"\nset the following registry DWORD value:"
"\n"
"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
"\nset the following environment variable:"
"\n"
"\nDOTNET_LoaderHeapCallTracing=1"

davidwrighton · 2025-04-09T22:40:19Z

/ba-g the failing test is unrelated to this change and was fixed by Michal earlier today.

am11 · 2025-04-10T06:32:10Z

Copilot spotted a typo #114246 (comment).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli

LGTM, thank you!

janvorli · 2025-04-10T12:42:57Z

+ // and notify the user to provide more reserved mem.
+ _ASSERTE((dwSizeToCommit <= dwSizeToReserve) && "Loaderheap tried to commit more memory than reserved by user");
+
+ if (!fReleaseMemory)


The fReleaseMemory is always set to TRUE at this point.

I'll fix that. Thanks.

janvorli · 2025-04-10T12:46:00Z

+
+ INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
+
+ // For interleaved heaps, further allocations will start from the newly committed page as they cannot


A nit - maybe remove the "For interleaved heap, " from multiple comments here, that was only usefull originally when the code for interleaved heaps stuff was intertwined with the other code.

I'll need to revisit these comments with the page remapping logic in my next PR. I'll address this then.

jkotas · 2025-04-12T03:20:50Z

-//
-// Caller is responsible for synchronization. ExplicitControlLoaderHeap is
-// not multithread safe.
+// The LoaderHeap is the black-box heap and has a Backout() method but none


This comment does not match InterleavedLoaderHeap

jkotas · 2025-04-12T03:23:54Z


 protected:
- void *UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, size_t dwCodeSize, DWORD dwCodeAlignment, size_t dwReserveForJumpStubs);
+ // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:25:53Z

- void SetReservedRegion(BYTE* dwReservedRegionAddress, SIZE_T dwReservedRegionSize, BOOL fReleaseMemory)
+
+public:
+ // This frees memory allocated by AllocMem. It's given this horrible name to emphasize


Suggested change
// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
// This frees memory allocated by AllocStub. It's given this horrible name to emphasize

jkotas · 2025-04-12T03:27:20Z

+#define DONOT_DEFINE_ETW_CALLBACK
+#include "eventtracebase.h"
+
+#ifndef DACCESS_COMPILE


Move the file to (non-DAC) UTILCODE_SOURCES in CMakeLists.txt instead of end-to-end ifdef

jkotas · 2025-04-12T03:28:51Z

+ m_codePageGenerator = codePageGenerator;
+}
+
+// ~LoaderHeap is not synchronised (obviously)


Suggested change
// ~LoaderHeap is not synchronised (obviously)

jkotas · 2025-04-12T03:30:45Z

+
+ size_t dwSize = dwRequestedSize;

+ // Interleaved heap cannot ad any extra to the requested size


Suggested change
// Interleaved heap cannot ad any extra to the requested size

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

davidwrighton added 13 commits January 26, 2024 13:38

Split explicit control heap from normal loader heaps
6d51df4

Finish splitting up loader heap code
a4d4dad

More splitting up of LoaderHeap and ExplicitControlLoaderHeap
f8962a0

Break LoaderHeap codebase into multiple files
08d4d3b

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
2ae1527
…_heap_implementations

Follow change from merge
15d776a

Current Progress
332d992

It builds now
e2c216a

Removed unnecessary ExplicitControlLoaderHeapBlock concept
a362f38

Move to having the right set of methods in the various files. Notably…
d00bb82
…, the free block handling is NOT a part of the interleaved heap as it didn't work, and wasn't actually hooked up.

Move field initialization up into the base classes as much as possible
5165ea5

Change stub allocation api from AllocAlignedMem to AllocStub
cdb66ef

Move all of the shared code back into LoaderHeap.cpp where its not sh…
1631405
…ared anymore Add support for a free list that actually works in the stub heaps

ghost added the area-VM-coreclr label Apr 3, 2025

dotnet-policy-serviceBot assigned davidwrighton Apr 3, 2025

davidwrighton added 2 commits April 3, 2025 16:33

Merge branch 'main' of github.com:dotnet/runtime into split_up_loader…
1647326
…_heap_implementations

Fix build breaks
d53ed49

build-analysisBot mentioned this pull request Apr 5, 2025
NativeAOT failure in System.Reflection.Tests.GetTypeTests.GetType_InvalidAssemblyName #114288
Closed

Update comments and fix at least 1 of the musl build breaks
cf91d74

build-analysisBot mentioned this pull request Apr 7, 2025
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open
3 tasks

davidwrighton changed the title ~~[DRAFT] Split up loader heap implementations~~Split up loader heap implementationsApr 8, 2025

davidwrighton marked this pull request as ready for review April 8, 2025 21:57

CopilotAI review requested due to automatic review settings April 8, 2025 21:57

CopilotAI reviewed Apr 8, 2025
View reviewed changes

Comment threadsrc/coreclr/vm/loaderallocator.hpp Outdated

jkotas reviewed Apr 9, 2025
View reviewed changes

Comment threadsrc/coreclr/utilcode/loaderheap_shared.cpp

jkotas reviewed Apr 9, 2025
View reviewed changes

Address feedback
14df448

davidwrighton requested a review from janvorli April 9, 2025 22:23

davidwrighton mentioned this pull request Apr 9, 2025
Create a single copy of stub templates #114462
Merged

davidwrighton requested a review from jkotas April 10, 2025 00:02

Update src/coreclr/vm/loaderallocator.hpp
fbba5a0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

janvorli approved these changes Apr 11, 2025
View reviewed changes

Remove nothion of non-released blocks from the InterleavedLoaderHeap
a0a4cdc

jkotas reviewed Apr 12, 2025
View reviewed changes

davidwrighton merged commit e6f38d7 into dotnet:mainApr 12, 2025

github-actionsBot locked and limited conversation to collaborators May 13, 2025

-"\nset the following registry DWORD value:"
-"\n"
-"\nHKLM\\Software\\Microsoft\\.NETFramework\\LoaderHeapCallTracing = 1"
+"\nset the following environment variable:"
+"\n"
+"\nDOTNET_LoaderHeapCallTracing=1"


		INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)

		// For interleaved heaps, further allocations will start from the newly committed page as they cannot

	// This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
	// This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize

	// This frees memory allocated by AllocMem. It's given this horrible name to emphasize
	// This frees memory allocated by AllocStub. It's given this horrible name to emphasize


		size_t dwSize = dwRequestedSize;

		// Interleaved heap cannot ad any extra to the requested size

Uh oh!

Conversation

davidwrighton commented Apr 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-serviceBot commented Apr 3, 2025

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwrighton commented Apr 9, 2025

Uh oh!

am11 commented Apr 10, 2025

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

davidwrighton commented Apr 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-serviceBot commented Apr 3, 2025

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwrighton commented Apr 9, 2025

Uh oh!

am11 commented Apr 10, 2025

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwrighton commented Apr 3, 2025•
edited
Loading

davidwrighton commented Apr 3, 2025•
edited
Loading

davidwrighton commented Apr 3, 2025•
edited
Loading