Perf followup for Pinned Object Heap

[VSadov]

A follow up for items tracked by #13739

Verified, adjusted or otherwise addressed the following items:

• NUM_POH_ALIST
  Since POH covers whole possible range of object sizes, we need a few more buckets than in SOH or LOH - to cover combined SOH and LOH range.
• static_data static_data_table
  POH has behaviors similar to LOH here - For example it is not very likely that GC will result in reducing the generation size (since there is no compaction and objects are relatively long lived).
  LOH tuning seems to be reasonable enough.
• initial allocation sizes (also min segment size)
  I did not see obvious reasons for the above to be different from LOH.
  It is possible we could be ok with lower initial/minimum segment size, but that seems to be making very little difference, since this is not about committed size. We commit in smaller chunks (2 OS pages) anyways.
• size_t gc_heap::get_uoh_seg_size (size_t size)
  Same as above.

Also looked in debugger and VTune for any obvious perf issues.
Apart from few minor tweaks, it all looked as expected.

[Maoni0]

unsigned int first_suitable_bucket(size_t size) [](start = 4, length = 47)

have you actually observed any perf difference doing this? I doubt it.

[VSadov]

On a microbenchmark doing a lot of POH allocations this becomes noticeable. I saw about 10% improvement from these changes.

The hot spots that I saw: taking msl lock, iterating through buckets, splitting a free chunk in two (when chunk is big, make_unused_array may cause cache misses)
The others are hard to avoid or improve though. This one was fairly easy.

[Maoni0]

can you share your benchmark? what sort of perf testing were done on this in general? are you running the default perf scenarios with the infra? this new scenario should be added to gcperfsim.

---

In reply to: 399581040 [](ancestors = 399581040)

[VSadov]

An example:

    private static void AllocateUninitializedPinned()
    {
        System.Console.Write("AllocateUninitializedPinned: ");
        var r = new Random(1234);
        Stopwatch sw = Stopwatch.StartNew();
        for (int i = 0; i < iters; i++)
        {
            int size = r.Next(2, 10000);
            var arr = GC.AllocateUninitializedArray<long>(size, pinned: true);
            Add(arr);   // noop when not retaining, store in a static list when do
        }

        sw.Stop();
        System.Console.WriteLine(sw.ElapsedMilliseconds);
    }

I had a variety of these - with not retaining arrays, retaining until after the iteration is over, retaining randomly in an array

like:

    private static void Add(long[] arr)
    {
        if (r.Next(100) > 50)
            staticArray[arr.Length & 2047] = arr;
    }

The not retaining anything scenario was the most sensitive to the cost of the allocation codepath.

[VSadov]

I ran the standard infra with these changes, but I mostly see noise. If there are improvements, they are too small in the scenarios measured.

Yes, we should add POH scenarios to infra. At least for regression testing.
We may need to settle on what would be the scenario too look at - how big allocations are. How much is retained, etc.
If we track scenarios for LOH, POH scenarios could be very similar, except for size.

[Maoni0]

LOH and POH are not necessarily that similar - LOH is compacted by default for hardlimit and POH can't be compacted; LOH has in general much larger object sizes. let's sync up next week on what scenarios we want to test.

[VSadov]

Yes. For regression testing we need specific scenarios.

The tweaks I did here were just for something that stood out from looking at the code and VTune.

[Maoni0]

what's the justification of doing this? also I would not do this with this PR.

[VSadov]

We take locks very frequently and they are nearly always uncontended. The codepath that handles the contention is very large. It seems wasteful to inline that into callers when the code is not expected to run.

[VSadov]

I tried to reduce the cost of taking these locks in most common scenarios. These locks are particularly expensive in POH allocations.

[VSadov]

@Maoni0 - I think I have addressed all the PR concerns. Please take a look. Thanks!

[Maoni0]

this should be 15

[VSadov]

Right!!! Because it is the highest bit in 64*1024 - 1, (zero-based as in BitScanReverse). I'll add a comment as well.

[Maoni0]

the rest LGTM!

[ghost]

Hello @VSadov!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[Maoni0]

LGTM!