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

JIT: see if guarded devirtualization for EqualityComparer methods pays off #9028

Closed
AndyAyersMS opened this issue Sep 27, 2017 · 3 comments
Closed
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 optimization tenet-performance Performance related issue
Milestone

Comments

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Sep 27, 2017

The changes for devirtualizing the EqualityComparer<T>.Default calls (dotnet/coreclr#14125) don't kick in in cases where the user can optionally supply a custom comparer -- see for instance Dictionary<TKey, TValue> and many other generic collection types.

Since the jit can now determine the type of the default comparer, it might be worthwhile for the jit to insert a runtime test to see if a particular IEqualityComparer<T> typed object is actually the default comparer type for T, and if so, devirtualize and inline when the test succeeds.

I have a partial prototype of the changes needed here: master...AndyAyersMS:GuardedDevirtualization. This has most of the analysis needed to determine when the guarded devirtualization could be done cheaply enough that it is viable. However there are still some roadblocks to work out:

  • Marking the IEqualityComparer<T> methods as [Intrinsic] leads to odd errors in the runtime when generating marshalling stubs. Not clear yet what is going on. For now the prototype just checks all interface calls for this interface type, which is likely going to slow the jit down considerably.
  • The actual transformation can't be done in the current place(s) that impDevirtualizeCall is invoked, because the call may not yet be at top level. The rough idea here is to detect and defer this case, mark the original indirect call as an inline candidate, wait for it to get hoisted to top level, and then retry.

Update (Nov 2020): the guarded devirtualization transform is now available in the jit (see dotnet/coreclr#21270), but not enabled by default. We're still lacking the data needed to figure out which call sites would benefit, and at those sites, what class to guess for. PGO data can give us both of these.

Also note Dictionary<TKey, TValue> already contains explicit logic for accelerating the default equality comparer path (see dotnet/coreclr#15419), so opportunities for doing guarded devirtualization for objects with type IEqualityComparer<T> may not be as compelling. So a good first step here would be to find specific opportunities, likely classes like Dictionary where there's an optional user-overridable comparer supplied to the .ctor.

category:cq
theme:devirtualization
skill-level:expert
cost:medium

@AndyAyersMS
Copy link
Member Author

Have some data that indicates that default comparers seem to predominate, so if we can speed them up a bit without slowing down the non-default comparers we may have something.

Considering this for 2.1.

@AndyAyersMS
Copy link
Member Author

Probably not getting to this in time for 2.1, so moving it back to Future.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@AndyAyersMS AndyAyersMS mentioned this issue Oct 19, 2020
54 tasks
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@AndyAyersMS AndyAyersMS removed the JitUntriaged CLR JIT issues needing additional triage label Nov 3, 2020
@AndyAyersMS AndyAyersMS modified the milestones: Future, 6.0.0 Nov 3, 2020
@AndyAyersMS AndyAyersMS self-assigned this Nov 3, 2020
@AndyAyersMS
Copy link
Member Author

GDV is now enabled whenever there is suitable PGO data, for any sort of virtual or interface call.

So, closing this.

@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2021
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 optimization tenet-performance Performance related issue
Projects
Archived in project
Development

No branches or pull requests

3 participants