Use Pinned Object Heap for MemoryPool #21614

benaadams · 2020-05-08T13:18:42Z

Allocate in the Pinned Object Heap (new for .NET 5.0) rather than pushing to LOH

Also means Finalizer and GCHandle can be dropped from MemoryPoolSlab

VSadov · 2020-05-08T16:13:54Z

src/Shared/Buffers.MemoryPool/MemoryPoolSlab.cs

-        /// <summary>
-        /// This handle pins the managed array in memory until the slab is disposed. This prevents it from being
-        /// relocated and enables any subsections of the array to be used as native memory pointers to P/Invoked API calls.
-        /// </summary>


Nice to see that all this code can be removed.

VSadov · 2020-05-08T16:28:33Z

src/Shared/Buffers.MemoryPool/MemoryPoolSlab.cs

        /// </summary>
-        public bool IsActive => !_isDisposed;
+        public bool IsActive => PinnedArray != null;



I think IDisposable is no longer needed and PinnedArray can be a readonly field.

Is needed to switch off the Slab; which then means its blocks are thrown away rather than returned to the pool (i.e. to take them out of circulation)

Well sorta... since the pool doesn't really shrink; but if it did... 😉

VSadov · 2020-05-08T17:32:39Z

src/Shared/Buffers.MemoryPool/SlabMemoryPool.cs

+            {
+                fixed(byte* ptr = slab.PinnedArray)
+                {
+                    basePtr = (IntPtr)ptr;


Perhaps Marshal.UnsafeAddrOfPinnedArrayElement could be simpler here?

TIL :)

Done.

The alignment parameter of the pinned allocated doesn't look to be exposed? Otherwise could skip most of the following code also (which 4096 aligns the blocks)

That would be amazing

Hmm. Since pinned objects do not move, alignment would be fairly simple to arrange internally. (compacting would be tricky but that is not an issue)

I could just get objSize + 4096 bytes from the heap and then format that as 2 or 3 objects, with one being the actual result with aligned payload, which I would return, and others immediately becoming garbage, up for reuse.

That would be an API addition though, with all the process attached, as usual.

Can you drive that addition @VSadov ?

I will try. Alignment was already a part of the original "cadillac" proposal for AllocateArray. Having actual use case would make it easier to argue for the usefullness.

The challenge could be to scope this to just pinned allocations. More general support for aligned allocations is possible, but that is completely different cost.

benaadams · 2020-05-08T18:54:08Z

Squashed to retrigger CI

davidfowl · 2020-05-08T19:18:03Z

@VSadov do we have counters for this yet?

cc @sywhang

VSadov · 2020-05-08T19:21:26Z

Some diagnostics (like SOS stuff) is not yet aware of POH.
The ETW part is done though (dotnet/runtime#34549). You will see these in events like allocation ticks, heap stats, etc..

benaadams · 2020-05-08T19:24:55Z

GC Counters in RuntimeEventSource.cs? https://github.com/dotnet/runtime/blob/863d4583f34b3d84010308edb3dd66721f7696ee/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/RuntimeEventSource.cs#L18-L37

VSadov · 2020-05-08T19:35:34Z

POH is logically in Gen2, so most everything should just work. The only part that exposes SOH/LOH/POH distinction seems to be _lohSizeCounter. I guess something similar needs to be added for POH.

You can get that data though via GC.GetGenerationSize(4)

sywhang · 2020-05-08T19:37:06Z

I already have a work item assigned to me to add POH counters: dotnet/runtime#35424

Maoni0 · 2020-05-08T19:40:00Z

great to see this getting used. do you have a workload you could run and show some before vs after results?

davidfowl · 2020-05-08T19:50:53Z

great to see this getting used. do you have a workload you could run and show some before vs after results?

All of them. We'll see the graphs change but it would be good to get a sense for what metrics would potentially get better so we can monitor those.

benaadams · 2020-05-08T19:59:00Z

POH is logically in Gen2, so most everything should just work.

Would be good to expose it in a separate manner so it is can be exposed and monitored (as per page 6 on https://aka.ms/aspnet/benchmarks )

VSadov · 2020-05-08T20:22:29Z

POH like LOH is collected as a part of Gen2 (and generally in the background) thus no separate POH or LOH collection counts.

(technically POH has no references to other generations and could collect less frequently than Gen2, but not a lot of reasons for doing that)

For the size - yes, you can get a POH size chart next to the LOH size chart and would be nice to have. I assume once the counter is added.

Maoni0 · 2020-05-09T00:45:05Z

even without separate POH stats you can detect perf changes (if there's any diff that is) and they would show up as % time in GC and/or heap size (reduction in them is the goal of this feature; POH data is just there to help with perf investigation). the idea is if you used to basically allocate all your pinned objects at the beginning of the process with other long lived objects, using this should show virtually no difference. if you have the scenarios described at the beginning of the design doc those are what we expect to see benefits.

halter73 · 2020-05-09T20:35:53Z

@aspnet-hello benchmark

pr-benchmarks · 2020-05-09T20:40:27Z

Starting 'Default' pipelined plaintext benchmark with session ID '01ed222c5a184bfab34a72897ffb6611'. This could take up to 30 minutes...

pr-benchmarks · 2020-05-09T20:45:59Z

Baseline

Starting baseline run on '95a22085303f9230b23294782c0ecd5668343641'...
RequestsPerSecond:           766,703
Max CPU (%):                 99
WorkingSet (MB):             91
Avg. Latency (ms):           3.18
Startup (ms):                466
First Request (ms):          125.93
Latency (ms):                0.39
Total Requests:              11,525,455
Duration: (ms)               15,030
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             8,502
Published Size (KB):         120,701
SDK:                         5.0.100-preview.5.20258.4
Runtime:                     5.0.0-preview.5.20253.7
ASP.NET Core:                5.0.0-preview.5.20255.6

PR

Starting PR run on '722c207ededd0efe7cf49eb5c35e649ae191eb55'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 766,703 |      99 |          91 |              3.18 |          466 |            8502 |              120701 |             125.93 |         0.39 |      0 |  1.00 |
|       After | 785,197 |      99 |          86 |              3.12 |          461 |            5502 |              120701 |             130.65 |          0.3 |      0 |  1.02 |

benaadams · 2020-05-09T20:55:37Z

@aspnet-hello benchmark json

benaadams · 2020-05-09T20:56:05Z

Can it run an mvc test or something with higher allocations?

benaadams · 2020-05-09T20:57:05Z

@aspnet-hello benchmark http2

pr-benchmarks · 2020-05-09T21:00:36Z

Starting 'http2' pipelined plaintext benchmark with session ID '0dff5b4ef5ed4d1280b72bd92bba6a91'. This could take up to 30 minutes...

pr-benchmarks · 2020-05-09T21:07:45Z

Baseline

Starting baseline run on '95a22085303f9230b23294782c0ecd5668343641'...
RequestsPerSecond:           246,887
Max CPU (%):                 90
WorkingSet (MB):             202
Avg. Latency (ms):           1.88
Startup (ms):                468
First Request (ms):          172.18
Latency (ms):                0.38
Total Requests:              3,703,308
Duration: (ms)               15,010
Socket Errors:               0
Bad Responses:               0
Build Time (ms):             7,502
Published Size (KB):         120,694
SDK:                         5.0.100-preview.5.20251.2
Runtime:                     5.0.0-preview.5.20253.7
ASP.NET Core:                5.0.0-preview.5.20255.6

PR

Starting PR run on '722c207ededd0efe7cf49eb5c35e649ae191eb55'...
| Description |     RPS | CPU (%) | Memory (MB) | Avg. Latency (ms) | Startup (ms) | Build Time (ms) | Published Size (KB) | First Request (ms) | Latency (ms) | Errors | Ratio |
| ----------- | ------- | ------- | ----------- | ----------------- | ------------ | --------------- | ------------------- | ------------------ | ------------ | ------ | ----- |
|      Before | 246,887 |      90 |         202 |              1.88 |          468 |            7502 |              120694 |             172.18 |         0.38 |      0 |  1.00 |
|       After | 247,311 |      90 |         204 |              2.65 |          459 |            5502 |              120694 |             182.52 |          0.4 |      0 |  1.00 |

pr-benchmarks · 2020-05-09T21:07:46Z

Starting 'json' pipelined plaintext benchmark with session ID '8bc6fb7cd8304755bbf9152ba5892e24'. This could take up to 30 minutes...

pr-benchmarks · 2020-05-09T21:07:48Z

Baseline

stdout: 
stderr: System.IO.InvalidDataException: Job file 8bc6fb7cd8304755bbf9152ba5892e24.kestrel-pipelined-plaintext.json doesn't include a top-level 'json' property for the specified scenario.
   at JobConsumer.Program.GetBuildInstructions(FileInfo processingFile) in /app/src/JobConsumer/Program.cs:line 292
   at JobConsumer.Program.BenchmarkPR(FileInfo processingFile, String session) in /app/src/JobConsumer/Program.cs:line 142
   at JobConsumer.Program.<>c__DisplayClass25_0.<<Main>b__0>d.MoveNext() in /app/src/JobConsumer/Program.cs:line 123

PR

halter73 · 2020-05-09T22:26:50Z

Thanks!

Maoni0 · 2020-05-09T23:32:42Z

what does "Build Time" mean?

davidfowl · 2020-05-10T00:48:34Z

The benchmarking infrastructure builds the PR.

benaadams force-pushed the pinned-heap branch from 51edcbd to 55ef777 Compare May 8, 2020 13:28

VSadov reviewed May 8, 2020

View reviewed changes

Pilchie added the area-servers label May 8, 2020

VSadov reviewed May 8, 2020

View reviewed changes

davidfowl approved these changes May 8, 2020

View reviewed changes

Use Pinned Object Heap for MemoryPool

722c207

benaadams force-pushed the pinned-heap branch from 223df4a to 722c207 Compare May 8, 2020 18:53

Maoni0 approved these changes May 8, 2020

View reviewed changes

VSadov approved these changes May 8, 2020

View reviewed changes

halter73 approved these changes May 9, 2020

View reviewed changes

halter73 merged commit a410ed4 into dotnet:master May 9, 2020

benaadams deleted the pinned-heap branch May 9, 2020 22:33

amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023

Use Pinned Object Heap for MemoryPool #21614

Use Pinned Object Heap for MemoryPool #21614

Uh oh!

Conversation

benaadams commented May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

VSadov May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

benaadams commented May 8, 2020

Uh oh!

davidfowl commented May 8, 2020

Uh oh!

VSadov commented May 8, 2020

Uh oh!

benaadams commented May 8, 2020

Uh oh!

VSadov commented May 8, 2020

Uh oh!

sywhang commented May 8, 2020

Uh oh!

Maoni0 commented May 8, 2020

Uh oh!

davidfowl commented May 8, 2020

Uh oh!

benaadams commented May 8, 2020

Uh oh!

VSadov commented May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Maoni0 commented May 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

halter73 commented May 9, 2020

Uh oh!

pr-benchmarks bot commented May 9, 2020

Uh oh!

pr-benchmarks bot commented May 9, 2020

Baseline

PR

Uh oh!

benaadams commented May 9, 2020

Uh oh!

benaadams commented May 9, 2020

Uh oh!

benaadams commented May 9, 2020

Uh oh!

pr-benchmarks bot commented May 9, 2020

Uh oh!

pr-benchmarks bot commented May 9, 2020

Baseline

PR

Uh oh!

pr-benchmarks bot commented May 9, 2020

Uh oh!

pr-benchmarks bot commented May 9, 2020

Baseline

PR

benaadams commented May 8, 2020 •

edited

Loading

VSadov May 8, 2020 •

edited

Loading

VSadov commented May 8, 2020 •

edited

Loading

Maoni0 commented May 9, 2020 •

edited

Loading