Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize out write barriers for fields in ref-like structs #9512

Closed
jkotas opened this issue Jan 5, 2018 · 10 comments · Fixed by #103503
Closed

Optimize out write barriers for fields in ref-like structs #9512

jkotas opened this issue Jan 5, 2018 · 10 comments · Fixed by #103503
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged JitUntriaged CLR JIT issues needing additional triage optimization tenet-performance Performance related issue
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Jan 5, 2018

From dotnet/coreclr#15745 (comment) :

Hmm, I wonder if there are sufficient guarantees that by ref like types only live on the stack for the JIT to be able to eliminate GC write barriers when writing to fields of such types.

Yes, there are. It would be nice optimization to have.

cc @mikedn

category:cq
theme:barriers
skill-level:expert
cost:small

@mikedn
Copy link
Contributor

mikedn commented Jan 6, 2018

And does a byref to a ref-like type serve any useful purpose?

It seems that it could be converted to an unmanaged pointer because it doesn't need to be reported to the GC. Though it looks like attempting to write an object reference to an unmanaged pointer makes the JIT emit a checked write barrier rather than no barrier, hmm...

@mikedn
Copy link
Contributor

mikedn commented Jan 6, 2018

An example containing 2 cases where the GC write barrier could be eliminated if the write location is known to be on the stack:

ref struct RefLike
{
    public object o;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public void NotInlined(object o)
    {
        this.o = o; // GC WB, the JIT currently assumes that `this` can be on the GC heap
    }

    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public void Inlined(object o)
    {
        this.o = o; // No GC WB if the method is inlined and `this` is on the stack.
    }
}

public static void Main()
{
    RefLike r = new RefLike();
    Test(new object(), ref r);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Test(object o, ref RefLike ar)
{
    RefLike lr = new RefLike();
    lr.o = o; // No GC WB
    lr.Inlined(o); // No GC WB
    lr.NotInlined(o); // GC WB inside NotInlined
    ar.o = o; // GC WB, the JIT currently assumes that `ar` can point into the GC heap 
}

@mikedn
Copy link
Contributor

mikedn commented Jan 6, 2018

I tried a quick and dirty implementation to see what happens. I wasn't expecting to see anything in the jit-diffs but it turns out that there's already a case like this in corelib. A bunch of GC write barriers disappeared from DateTimeParse.ParseByFormat due to DateTimeResult being a ref struct.

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2018

Another example - a non ref struct inside a ref struct:

struct NotRefLike
{
    public object o;
}

ref struct RefLike
{
   public object o;
   public NotRefLike n;
   ...
}
...
lr.n.o = o; // No GC WB already, it's known to be a local variable
ar.n.o = o; // No GC WB with my hack. Fortunately the JIT transforms ar.n.o into a single indir.

lr.n.FooBar(); // Of course, any not inlined method of NotRefLike will have GC WBs.

ref object ro = ref lr.n.o;
Console.WriteLine();
ro = o; // GC WB unfortunately. Not directly related to ref-like stuff,
        // the JIT just doesn't figure out that ro points into the stack.

  

@benaadams
Copy link
Member

ref struct BufferReader<TSequence> in Pipelines/Buffers and its 2x non ref struct Position is an example where this would help dotnet/corefxlab#2046 (comment)

@mikedn
Copy link
Contributor

mikedn commented Jan 12, 2018

Thanks for the example, copying structs like Position is not something that I tried yet.

@mikedn
Copy link
Contributor

mikedn commented Jan 14, 2018

Yep, copying structs require a bit more hacking. Currently copying a struct like Position generates something like

       488D7118             lea      rsi, bword ptr [rcx+24]
       488D7908             lea      rdi, bword ptr [rcx+8]
       E8A1316B5F           call     CORINFO_HELP_ASSIGN_BYREF
       48A5                 movsq

I can change it to

       488D7118             lea      rsi, bword ptr [rcx+24]
       488D7908             lea      rdi, bword ptr [rcx+8]
       48A5                 movsq
       48A5                 movsq

I'm not sure if the use of 2 movsq to copy 16 bytes is a good idea, these instructions do not have a reputation for performance. But they surely keep the code small and avoid issues with GC. The ideal solution to copy 16 bytes would probably use SSE load/store but GC won't be happy with it since it cannot "see" the object reference stored in the first 8 bytes of the temporary XMM register.

@benaadams
Copy link
Member

Example dotnet/aspnetcore#7674 resolved via inlining

@benaadams
Copy link
Member

benaadams commented Feb 23, 2019

Even uses CORINFO_HELP_CHECKED_ASSIGN_REF when the ref struct is fully inlined (dotnet/corefx@a3cb00f) e.g.

private ref struct ValueSegmentList
{
    public BufferSegment Segment00;
    public BufferSegment Segment01;
    public BufferSegment Segment02;
    public BufferSegment Segment03;
    public BufferSegment Segment04;
    public BufferSegment Segment05;
    public BufferSegment Segment06;
    public BufferSegment Segment07;
    public BufferSegment Segment08;
    public BufferSegment Segment09;
    public BufferSegment Segment10;
    public BufferSegment Segment11;
    public BufferSegment Segment12;
    public BufferSegment Segment13;
    public BufferSegment Segment14;
    public BufferSegment Segment15;
}

with

ValueSegmentList segmentsToReturn = default;
ref var startSegment = ref segmentsToReturn.Segment00;
int count = 0;
do
{
    BufferSegment next = from.NextSegment;
    Debug.Assert(next != null || toExclusive == null);

    from.ResetMemory();

    if ((uint)count < (uint)SegmentPoolSize)
    {
        // Store in temporary list while preforming expensive resets
        Unsafe.Add(ref startSegment, count) = from;
        count++;
    }

    from = next;
} while (from != toExclusive);
G_M48697_IG02:
       lea      rcx, bword ptr [rbp-B8H]
       vxorps   xmm0, xmm0
       vmovdqu  qword ptr [rcx], xmm0
       vmovdqu  qword ptr [rcx+16], xmm0
       vmovdqu  qword ptr [rcx+32], xmm0
       vmovdqu  qword ptr [rcx+48], xmm0
       vmovdqu  qword ptr [rcx+64], xmm0
       vmovdqu  qword ptr [rcx+80], xmm0
       vmovdqu  qword ptr [rcx+96], xmm0
       vmovdqu  qword ptr [rcx+112], xmm0
       lea      r14, bword ptr [rbp-B8H]
       xor      r15d, r15d

G_M48697_IG03:
       mov      r12, gword ptr [rsi+40]
       mov      rcx, rsi
       call     BufferSegment:ResetMemory():this
       cmp      r15d, 16
       jae      SHORT G_M48697_IG04
       movsxd   rdx, r15d
       lea      rcx, bword ptr [r14+8*rdx]
       mov      rdx, rsi
       call     CORINFO_HELP_CHECKED_ASSIGN_REF  ; checked assign
       inc      r15d

G_M48697_IG04:
       mov      rsi, r12
       cmp      rsi, rdi
       jne      SHORT G_M48697_IG03

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@markples
Copy link
Member

#86820, #89064, and #89116 are related (for struct copies)

@EgorBo EgorBo self-assigned this Jun 15, 2024
@EgorBo EgorBo modified the milestones: Future, 9.0.0 Jun 15, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jun 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged JitUntriaged CLR JIT issues needing additional triage optimization tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants