Change BulkMoveWithWriteBarrier to be GC suspension friendly

[jkotas]

No description provided.

[jkotas]

Micro-benchmark results:

// Mean pause time: 
// Baseline: 32.9ms
// This change: 21ms
static void Main()
{
    new Thread(() =>
    {
        var src = new object[10_000_000]; 
        var dst = new object[10_000_000]; 
        for (;;) Array.Copy(src, dst, src.Length);
    }).Start();
    for (;;) { GC.Collect(); Thread.Sleep(1); }
}

// Mean pause time: 
// Baseline: 33.1ms
// This change: 0.8ms
static void Main()
{
    new Thread(() =>
    {
        var a = new byte[100_000_000];
        for (;;) a.Clone();
    }).Start();
    for (;;) { GC.Collect(); Thread.Sleep(1); }
}

[GrabYourPitchforks]

The managed part seems sound to me, but I'm not familiar enough with QCalls and GC cooperative modes to comment on the native components.

[VSadov]

I would expect latencies in object Array.Copy to improve more if that was solely due to copying. Any idea what is the long pole now? Suspension itself?

The byte Clone benchmark would not have anything to suspend most of the time and thus shorter pauses?

[VSadov]

Could be interesting to measure impact on GC.Collect(0) as well.

[VSadov]

I think it could be simpler if “chunking” happens on the managed side. Then it would be regular fcalls without polling and frames. (move itself does not trigger or throw)

Bulk w barrier move would just need to assert it is not asked to move too much at once.

[jkotas]

It may be simpler, but less efficient. It can either add a new frame that does the chunking (throughput overhead - for small block sizes in particular); or aggressive inline the chunking into every callsite to avoid the frame (code side overhead). Which tradeoff would you pick if the chunking happens on the managed side?

[VSadov]

I was thinking of something like:

ManagedHelperProbablyInlineable()
{
     // get refs, byte count and whether need to copy backwards
     nint count = . . .;
     if (count < CHUNK_SIZE)
     {
           NativeHelper(ref src, ref dst, count, backwards);
     }
     else
     {
           ManagedChunkingHelperDoNotInline(mt, ref src, ref dst, count, backwards);
     }
}

ManagedChunkingHelperDoNotInline(MethodTable *mt, ref byte src, ref byte dst, nint count, bool backwards)
{
       int elementSize = . . . ;
       while(count)
       {
              int chunk = Align(min(count, chunkSize), elementSize);
              NativeHelper(src, dst, chunk , backwards);
              count -= chunk;
              src = . . .;
              dst = . . .;
       }
}

[jkotas]

I would expect latencies in object Array.Copy to improve more if that was solely due to copying. Any idea what is the long pole now? Suspension itself?

The Array.Copy example is copying object references in Gen2. The GC (whether it is Gen0 GC or Gen2 GC) has to visit these modified references. 21ms pause is cost to visit these modified references.

In the baseline, the pause time time is 12ms waiting for threads to suspend and 21ms GC doing work. With this change, it is just 21ms GC doing work.

I have picked this example specifically to show that this change helps, but copying large arrays of object references has still less than ideal performance characteristics.

[VSadov]

The sample copies null references. And even if they were objects, they'd be old. I did not realize we update cards unconditionally. Yes, that introduces a lot of roots to scan and explains the remaining pause.

Checking for copying null pointers is not too hard, but may not be a typical enough scenario. Checking whether writes are cross-generational (even in a conservative way) is not an easy thing to fix right now. Especially in server GC.
Something to think about....

[jkotas]

I think it could be simpler if “chunking” happens on the managed side

Pushed update with this change. @VSadov How does the new version look?

[VSadov]

Looks nice!!
Array elements are always aligned when contain references? (I cant think of a case otherwise, but just in case..)

[jkotas]

Array elements are always aligned when contain references?

Yes.

[VSadov]

65K - not too big?

[jkotas]

65k should be less than 50 microseconds. I can make it smaller.

[jkotas]

/azp run coreclr-ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).