Introducing Pinned Object Heap #32283

VSadov · 2020-02-14T05:57:20Z

Not exposed through any API yet.
Has some extra test code for stress coverage. Will be removed before merging.

AaronRobinsonMSFT · 2020-02-14T06:18:19Z

src/coreclr/src/gc/gc.cpp

@@ -2427,6 +2431,9 @@ static static_data static_data_table[latency_level_last - latency_level_first +
        // gen2
        {256*1024, SSIZE_T_MAX, 200000, 0.25f, 1.2f, 1.8f, 100000, 100},
        // loh
+        {3*1024*1024, SSIZE_T_MAX, 0, 0.0f, 1.25f, 4.5f, 0, 0},
+        // loh


Should this be poh? #Resolved

yes, should be poh.
Initial tuning is copied from loh and may be adjusted further, if needed. The comment should say poh though. #Resolved

VSadov · 2020-02-14T19:29:22Z

I think this is ready for the review.

VSadov · 2020-02-14T19:36:34Z

In this iteration I am sending LOH allocations with 50/50 chance to POH - for testing purposes.
That hack will be removed before merging, but it is a good testing strategy for something not yet exposed via APIs.

All tests have passed, except lohpin, which is expected to fail if the feature works correctly (since the test measures compaction and we are preventing that when allocation goes into POH).
Failure in lohpin actually demonstrates that POH works as expected.
long running GC tests passed on all platforms (includes Reliability Framework with default settings)
I have run stress locally with:

set COMPlus_GCSegmentSize=4000000
set COMPlus_gcServer=1

that passed too with Chk and Ret.

I am fairly confident now that POH works and caused no regressions.

Maoni0 · 2020-02-14T20:16:35Z

great! I'll be reviewing this over the weekend; I already looked at the changes before so I would expect only small changes.

VSadov · 2020-02-14T21:47:17Z

@Maoni0 Thanks! The changes are indeed small, ~200 lines, and fairly mechanical too.

Maoni0 · 2020-02-17T05:00:33Z

src/coreclr/src/gc/gc.cpp

@@ -20,6 +20,7 @@
 #include "gcpriv.h"

 #define USE_INTROSORT
+#define ALLOW_REFERENCES_IN_POH


this would be better in gcpriv.h which has all the feature related defines. the #define's in gc.cpp are purely implementation related, ie, you wouldn't expect someone to define or define them to enable a feature. #Resolved

Maoni0 · 2020-02-17T05:03:12Z

src/coreclr/src/gc/gc.cpp

-        {3*1024*1024, SSIZE_T_MAX, 0, 0.0f, 1.25f, 4.5f, 0, 0}
+        {3*1024*1024, SSIZE_T_MAX, 0, 0.0f, 1.25f, 4.5f, 0, 0},
+        // poh
+        // TODO: using same numbers as for LOH. May need to tune this (see: https://github.com/dotnet/runtime/issues/13739)


this will need to be tuned so you could just say "TODO: tuning PR_link_from_GH #Resolved

Maoni0 · 2020-02-17T05:03:41Z

src/coreclr/src/gc/gc.cpp

@@ -2427,6 +2431,9 @@ static static_data static_data_table[latency_level_last - latency_level_first +
        // gen2
        {256*1024, SSIZE_T_MAX, 200000, 0.25f, 1.2f, 1.8f, 100000, 100},
        // loh
+        {3*1024*1024, SSIZE_T_MAX, 0, 0.0f, 1.25f, 4.5f, 0, 0},
+        // loh
+        // TODO: using same numbers as for LOH. May need to tune this (see: https://github.com/dotnet/runtime/issues/13739)


// TODO: using same numbers [](start = 8, length = 27)

same here. #Resolved

Maoni0 · 2020-02-17T05:28:17Z

src/coreclr/src/gc/gc.cpp

-            dprintf (3, ("Relocating cross generation pointers for large objects on heap %d", heap_number));
-            mark_through_cards_for_uoh_objects(&gc_heap::relocate_address, loh_generation, TRUE THIS_ARG);
+            dprintf (3, ("Relocating cross generation pointers for uoh objects on heap %d", heap_number));
+            for(int i = uoh_start_generation; i < total_generation_count; i++)


for(int [](start = 12, length = 7)

nit - formatting, needs a space between for and ( #Resolved

Maoni0 · 2020-02-17T05:29:53Z

src/coreclr/src/gc/gc.cpp

-                dprintf(3, ("Relocating cross generation pointers for large objects on heap %d", hp->heap_number));
-                hp->mark_through_cards_for_uoh_objects(&gc_heap::relocate_address, loh_generation, TRUE THIS_ARG);
+                dprintf(3, ("Relocating cross generation pointers for uoh objects on heap %d", hp->heap_number));
+                for(int i = uoh_start_generation; i < total_generation_count; i++)


for(int i [](start = 16, length = 9)

nit - same formatting, needs space #Resolved

Maoni0 · 2020-02-17T05:36:03Z

src/coreclr/src/gc/gc.cpp

@@ -33472,6 +33575,16 @@ CObjectHeader* gc_heap::allocate_large_object (size_t jsize, uint32_t flags, int
    return obj;
 }

+CObjectHeader* gc_heap::allocate_large_object (size_t jsize, uint32_t flags, int64_t& alloc_bytes)


why do we need these 2 methods at all instead of just calling allocate_uoh_object and passing in the right args? #Resolved

Maoni0 · 2020-02-17T05:38:32Z

src/coreclr/src/gc/gcpriv.h

@@ -3982,6 +3997,16 @@ class gc_heap
    PER_HEAP
    alloc_list gen2_alloc_list[NUM_GEN2_ALIST-1];

+// TODO: using same numbers as for LOH. May need to tune this (see: https://github.com/dotnet/runtime/issues/13739)


// TODO: using same numbers as for LOH. May need to tune [](start = 0, length = 57)

again, this will need to be tuned so I would not say "May need to tune" #Resolved

Maoni0 · 2020-02-17T05:39:30Z

LGTM aside from some minor comments above! feel free to merge after those are addressed.

VSadov · 2020-02-17T15:26:49Z

Thanks!!

richlander · 2020-05-16T22:31:10Z

Adding a link to the design doc for people looking at this PR. I didn't see it in the PR.

https://github.com/dotnet/runtime/blob/master/docs/design/features/PinnedHeap.md

VSadov · 2020-05-16T23:15:18Z

Here is the other PR where POH was made available for use via public APIs:
#33526

Just for reference and to link these two PRs together.

Introducing Pinned Object Heap

3cfd1e8

jkotas added the area-GC-coreclr label Feb 14, 2020

AaronRobinsonMSFT reviewed Feb 14, 2020

View reviewed changes

VSadov changed the title ~~[WIP] Introducing Pinned Object Heap~~ Introducing Pinned Object Heap Feb 14, 2020

VSadov marked this pull request as ready for review February 14, 2020 19:28

VSadov requested a review from Maoni0 February 14, 2020 19:28

GrabYourPitchforks mentioned this pull request Feb 15, 2020

Allow to allocate known eternal objects (especially buffers) #4369

Closed

Maoni0 reviewed Feb 17, 2020

View reviewed changes

Maoni0 approved these changes Feb 17, 2020

View reviewed changes

VSadov added 2 commits February 16, 2020 22:13

PR feedback

712658e

reverted a test-only changes

ba4dcbb

VSadov merged commit e0a1cf9 into dotnet:master Feb 17, 2020

VSadov deleted the poh branch February 17, 2020 15:26

davmason mentioned this pull request Apr 21, 2020

Pinned object heap impact on profilers #35258

Closed

davmason mentioned this pull request May 21, 2020

Profiler Interest Group - Announcements #9305

Open

richlander mentioned this pull request Jun 6, 2020

Improving P95+ latency #37534

Closed

noahfalk mentioned this pull request Aug 5, 2020

Address Diagnostic impact of Pinned Object Heap dotnet/diagnostics#1408

Closed

18 tasks

ghost locked as resolved and limited conversation to collaborators Dec 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing Pinned Object Heap #32283

Introducing Pinned Object Heap #32283

VSadov commented Feb 14, 2020 •

edited

Loading

AaronRobinsonMSFT Feb 14, 2020 •

edited by VSadov

Loading

VSadov Feb 14, 2020 •

edited

Loading

VSadov commented Feb 14, 2020

VSadov commented Feb 14, 2020 •

edited

Loading

Maoni0 commented Feb 14, 2020

VSadov commented Feb 14, 2020

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 commented Feb 17, 2020 •

edited

Loading

VSadov commented Feb 17, 2020

richlander commented May 16, 2020

VSadov commented May 16, 2020

Introducing Pinned Object Heap #32283

Introducing Pinned Object Heap #32283

Conversation

VSadov commented Feb 14, 2020 • edited Loading

AaronRobinsonMSFT Feb 14, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

VSadov Feb 14, 2020 • edited Loading

Choose a reason for hiding this comment

VSadov commented Feb 14, 2020

VSadov commented Feb 14, 2020 • edited Loading

Maoni0 commented Feb 14, 2020

VSadov commented Feb 14, 2020

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 Feb 17, 2020 • edited by VSadov Loading

Choose a reason for hiding this comment

Maoni0 commented Feb 17, 2020 • edited Loading

VSadov commented Feb 17, 2020

richlander commented May 16, 2020

VSadov commented May 16, 2020

VSadov commented Feb 14, 2020 •

edited

Loading

AaronRobinsonMSFT Feb 14, 2020 •

edited by VSadov

Loading

VSadov Feb 14, 2020 •

edited

Loading

VSadov commented Feb 14, 2020 •

edited

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 Feb 17, 2020 •

edited by VSadov

Loading

Maoni0 commented Feb 17, 2020 •

edited

Loading