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-dasm-pmi - add explicit GC #164

Open
echesakov opened this issue Aug 29, 2018 · 3 comments
Open

jit-dasm-pmi - add explicit GC #164

echesakov opened this issue Aug 29, 2018 · 3 comments

Comments

@echesakov
Copy link
Contributor

On Linux (x64 and arm32) the following observed when there is a race between JIT compiling method and the finalizer thread (and the finalizer also got jitted).

@@ -120401,79 +120401,79 @@ G_M40565_IG02:
             movs    r4, 0
             movs    r2, 0
             str     r2, [sp+0x0c]
-            adr     r2, G_M40565_IG04
+            adr     r2, G_M40566_IG04
             str     r2, [sp+0x14]
             movs    r2, 0
             strb    r2, [r5+8]

-G_M40565_IG03:
+G_M40566_IG03:
             blx     r3

-G_M40565_IG04:
+G_M40566_IG04:
             movs    r3, 1
             strb    r3, [r5+8]
             movw    r3, 0xd1ff
             movt    r3, 0xd1ff
             ldr     r3, [r3]
             cmp     r3, 0
-            beq     SHORT G_M40565_IG05
+            beq     SHORT G_M40566_IG05
             movw    r12, 0xd1ff
             movt    r12, 0xd1ff
-; Assem b   l  y list i     ng for blxmethod     System.Gen2GcCallback:Finalize():this
-; Emitting BLENDED_CODE for generic ARM CPU
-; optimized code
-; r11 r12based fr      ame
-       ; f/ully/ int errupCORINFO_HELP_STOP_FOR_GCtibl
-e
+            blx     r12                // CORINFO_HELP_STOP_FOR_GC

-G_M40565_IG05:
-  ;          F inalmov sloc al va riabl e  assignments
-r3;
-, 0
-            str   ;    r3, [V00 thissp+         0x[V14]00,T00] (  7,  6   )     ref
- ->
-G_M[sp40565+_0xIG04]06 :
-  do-not -enre g    [      H]mov st h i s  class-hndr3
-; ,  1
-V01 loc0               [V     01,T06] (  3,  3   )     ref   ->  strb r1            class- hnd
-;# r3, [r5+V02 OutArgs8      ][V
-02    ]       (      1 ,   1   )ldr    lclBlk  (   0) r3[, sp[+0xsp00+]  0x
-08]
-            str     r3, [r5;+* 12]
-       V03 tmp1         [V03    ] (  0,   0   )         ref  ->  addz e r o -r ef   sp ,cla ss-h36
-            pop     {nr4,dr5
-,;r6 , r7,r8,r9V04 tmp2,         r10[V,04r11,T,02]pc} (
-  2,
-  4   ) ; Tot  byrefa l b y-tes o>f   code  r0  100 , pr  o l  og
-size ;  14 for method V05 tmp3DomainNeutralILStubClass:IL_STUB_PInvoke(int):int
-         ;[V =============05======,=====T===============03] (  2,  4   )     int  ->  ===== r3=  === ==== ==== ====
-   ld-addr-op

It was suggested by @AndyAyersMS and @BruceForstall to explicitly call GC (e.g. before PrepareMethod) so GC happens more predictably. It worked for me but this makes jit-dasm-pmi much slower - so it makes sense to add this as an option which can be specified in command line (say, --force_gc_collect)?

I have never seen this happening on Windows though.

@mikedn
Copy link
Contributor

mikedn commented Aug 29, 2018

I have never seen this happening on Windows though.

I saw it on Windows too. AFAIR it involved a CriticalFinalizerObject method rather than Gen2GcCallback but that's pretty much the same thing.

It worked for me but this makes jit-dasm-pmi much slower

Stupid idea of the day - create a finalizable object right at the start of PMI and have its finalizer block indefinitely so no other finalizers get to run.

You may also try to enable the Server GC, that one is more lazy with collection. Or try GC.TryStartNoGCRegion to prevent GC, though if PMI produces a lot of garbage that may not work so well.

@echesakov
Copy link
Contributor Author

echesakov commented Aug 30, 2018

How about this? I think we still need to GC.WaitForPendingFinalizers() before NoGCRegion

--- a/src/pmi/pmi.cs
+++ b/src/pmi/pmi.cs
@@ -254,7 +254,12 @@ abstract class PrepareBase : CounterBase
         try
         {
             DateTime startFunc = DateTime.Now;
+            GC.WaitForPendingFinalizers();
+            bool disallowFullBlockingGC = false;
+            GC.TryStartNoGCRegion(1024, disallowFullBlockingGC);
             System.Runtime.CompilerServices.RuntimeHelpers.PrepareMethod(method.MethodHandle);
+            GC.EndNoGCRegion();
             elapsedFunc = DateTime.Now - startFunc;
         }
         catch (System.EntryPointNotFoundException)

Also System.Runtime.CompilerServices.RuntimeHelpers.PrepareMethod is implemented in unmanaged code so I think we don't expect any managed garbage there?

This worked for me on Linux/x64 which doesn't prove it will always work ...

@mikedn
Copy link
Contributor

mikedn commented Aug 30, 2018

Also System.Runtime.CompilerServices.RuntimeHelpers.PrepareMethod is implemented in unmanaged code so I think we don't expect any managed garbage there?

I don't think being implemented in unmanaged code precludes GC allocations. But in the case of this particular method, yes, it's unlikely that it allocates. I would guess that just WaitForPendingFinalizers should be enough but it's probably good to keep TryStartNoGCRegion as well, provided that it doesn't have any negative side effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants