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

Remove code for GC Poll marking and insertion. #42664

Merged
merged 1 commit into from
Sep 25, 2020

Conversation

erozenfeld
Copy link
Member

This is a follow-up to
#13582 (comment)
and
#13582 (comment)

The code to insert gc polls was added in desktop for gc suspension not based
on hijaking. All platforms we target support hijaking so this code is not exercised
or tested. It also clutters other code and adds a bit of runtime overhead.
This change removes all that code.

There are minimal asm diffs because of a removed call to fgRenumberBlocks.

pin-icount reports a small but measurable throughput improvement when crossgen-ing SPC on x64: 0.07% (noise level is less than 0.01%).

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 24, 2020
@erozenfeld
Copy link
Member Author

x64 pmi framework diffs:

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  default jit

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 50341895
Total bytes of diff: 50341740
Total bytes of delta: -155 (-0.000% of base)
    diff is an improvement.

Top file regressions (bytes):
           2 : Microsoft.CodeAnalysis.VisualBasic.dasm (0.000% of base)

Top file improvements (bytes):
        -144 : System.Private.Xml.dasm (-0.004% of base)
         -13 : Microsoft.CodeAnalysis.CSharp.dasm (-0.000% of base)

3 total files with Code Size differences (2 improved, 1 regressed), 264 unchanged.

Top method regressions (bytes):
           2 (0.228% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - VisualBasicWarningStateMap:CreateWarningStateEntries(ImmutableArray`1):ref

Top method improvements (bytes):
        -144 (-3.643% of base) : System.Private.Xml.dasm - SchemaGraph:Depends(XmlSchemaObject,ArrayList):this
         -13 (-0.591% of base) : Microsoft.CodeAnalysis.CSharp.dasm - RetargetingSymbolTranslator:Retarget(NamedTypeSymbol,ubyte):NamedTypeSymbol:this

Top method regressions (percentages):
           2 (0.228% of base) : Microsoft.CodeAnalysis.VisualBasic.dasm - VisualBasicWarningStateMap:CreateWarningStateEntries(ImmutableArray`1):ref

Top method improvements (percentages):
        -144 (-3.643% of base) : System.Private.Xml.dasm - SchemaGraph:Depends(XmlSchemaObject,ArrayList):this
         -13 (-0.591% of base) : Microsoft.CodeAnalysis.CSharp.dasm - RetargetingSymbolTranslator:Retarget(NamedTypeSymbol,ubyte):NamedTypeSymbol:this

3 total methods with Code Size differences (2 improved, 1 regressed), 258658 unchanged.

@erozenfeld
Copy link
Member Author

@dotnet/jit-contrib @jkotas PTAL

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Contributor

@sandreenko sandreenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jit changes look good, thank you.

This is a follow-up to
dotnet#13582 (comment)
and
dotnet#13582 (comment)

The code to insert gc polls was added in desktop for gc suspension not based
on hijaking. All platforms we target support hijaking so this code is not exercised
or tested. It also clutters other code and adds a bit of runtime overhead.
This change removes all that code.

There are minimal asm diffs because of a removed call to `fgRenumberBlocks`.
@erozenfeld erozenfeld merged commit fef560c into dotnet:master Sep 25, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants