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

PGO: Optimize isinst with class probes #55325

Closed
EgorBo opened this issue Jul 8, 2021 · 4 comments
Closed

PGO: Optimize isinst with class probes #55325

EgorBo opened this issue Jul 8, 2021 · 4 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Jul 8, 2021

Example:

void Test(object o)
{
    if (o is Program)
    {
        Console.WriteLine();
    }
}

Current codegen:

G_M25341_IG01:
       sub      rsp, 40
G_M25341_IG02:
       mov      rcx, 0xD1FFAB1E
       call     CORINFO_HELP_ISINSTANCEOFCLASS
       test     rax, rax
       je       SHORT G_M25341_IG04
G_M25341_IG03:
       add      rsp, 40
       jmp      System.Console:WriteLine()
G_M25341_IG04:
       add      rsp, 40
       ret      
; Total bytes of code: 38

As you can see - we give up on inlined checks and just call the CORINFO_HELP_ISINSTANCEOFCLASS helper.
Ideally we could emit a fast-path check from a class probe. So it could be:

if (o->pMT == classProbe || call CORINFO_HELP_ISINSTANCEOFCLASS)

I wonder if such an improvement could optimize e.g. this hot-path in JsonSerializer:
image

where the converter is mostly ObjectDefaultConverter for most benchmarks (in real world apps likehood can be smaller)

@EgorBo EgorBo added the tenet-performance Performance related issue label Jul 8, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Jul 8, 2021
@EgorBo EgorBo added this to the 7.0.0 milestone Jul 8, 2021
@EgorBo
Copy link
Member Author

EgorBo commented Jul 8, 2021

cc @AndyAyersMS

@AndyAyersMS
Copy link
Member

For cases where the object is used after the test we may already have downstream class probes that would tell us the likely class. I wonder if we could leverage that somehow?

Ideally we'd probe each object's class just once instead of at each use. We're not set up to do that now as we don't understand how definitions and uses inter-relate.

@EgorBo
Copy link
Member Author

EgorBo commented Jul 8, 2021

For cases where the object is used after the test we may already have downstream class probes that would tell us the likely class. I wonder if we could leverage that somehow?

Good idea, so we can avoid more class probes in tier0

@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 8, 2021
@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Jul 19, 2021
@EgorBo
Copy link
Member Author

EgorBo commented Jun 20, 2022

Closed via
#65460
#65922
#69869

@EgorBo EgorBo closed this as completed Jun 20, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 20, 2022
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 tenet-performance Performance related issue
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants