Skip to content

JitInfoIsPopulated failed with after is not greater than before #55712

Closed

Description

@danmoseley

System.Runtime.Tests.JitInfoTests.JitInfoIsPopulated [Details] [History] [Helix Test Artifacts]

Exception Message
CompilationTime: after not greater than before! (after: 00:00:00.0257376, before: 00:00:00.0257376)
Expected: True
Actual: False
CallStack
at System.Runtime.Tests.JitInfoTests.JitInfoIsPopulated()
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
Pass Rate
Test has failed 40 out of 958 runs (4.18%)
Failing Configuration
net6.0-iOSSimulator-Release-x64-Mono_Release-OSX.1015.Amd64.Open

https://github.com/dotnet/runtime/runs/3071937514

Did the code run in less than 100ns? Seems unlikely that would pop up on the iOS simulator which is not our fastest configuration. So maybe a real bug?

Activity

  1. added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    on Jul 15, 2021
  2. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Jul 15, 2021
  3. danmoseley commented on Jul 15, 2021

    @danmoseley
    ContributorAuthor

    @josalem this is a new test you added. I see you special case the mono interpreter. Should the simulator also have this special case?

  4. danmoseley commented on Jul 15, 2021

    @danmoseley
    ContributorAuthor

    src/libraries/System.Runtime/tests/System/Runtime/JitInfoTests.cs

    21a7632#diff-b05d7e9d113484fa5e220a32499d986f133738f27ef161a664e93783e59a3afc

  5. danmoseley commented on Jul 15, 2021

    @danmoseley
    ContributorAuthor
  6. josalem commented on Jul 15, 2021

    @josalem
    Contributor

    I would expect iOS to not run this test due to the IsNotMonoAot conditional fact attribute. Is there a reason that isn't the case? Based on the behavior it looks like the same as the interpreter. Is the simulator running under the interpreter perhaps?

  7. danmoseley commented on Jul 15, 2021

    @danmoseley
    ContributorAuthor

    @steveisok can answer that.

  8. steveisok commented on Jul 15, 2021

    @steveisok
    Member

    @fanyang-mono what are your thoughts?

  9. lambdageek commented on Jul 15, 2021

    @lambdageek
    Member

    @steveisok What sets the MONO_AOT_MODE environment variable? I see in wasm it's set by the driver.c. I would have expected that something in eng/testing/tests.mobile.targets sets is for other configurations.

  10. fanyang-mono commented on Jul 15, 2021

    @fanyang-mono
    Member

    I took a look at the code. And this is my understanding. The failed check in the test is checking System.Runtime.JitInfo.GetCompilationTime(). The mono definition lives at

    private static long GetCompilationTimeInTicks(bool currentThread = false)
    {
    return currentThread ? 0 : (long)EventPipeInternal.GetRuntimeCounterValue(EventPipeInternal.RuntimeCounters.JIT_TICKS_IN_JIT);
    }

    It eventually queries the jit_time field of struct MonoJitStats, which only gets updated during JIT inside mono_jit_compile_method_inner
    start = mono_time_track_start ();
    cfg = mini_method_compile (method, opt, JIT_FLAG_RUN_CCTORS, 0, -1);
    gint64 jit_time = 0.0;
    mono_time_track_end (&jit_time, start);
    UnlockedAdd64 (&mono_jit_stats.jit_time, jit_time);

    Function mono_jit_compile_method_inner will only be executed during JIT. It won't be executed when it is interpreter mode. Thus, for interpreter mode, the test was actually getting the original garbage value of jit_time field. That's why it doesn't change before and after compile.

    Based on the error message, it seems that afterCompilationTime equals to beforeCompilationTime, which means the check PlatformDetection.IsMonoInterpreter failed. (I thought net6.0-iOSSimulator-Release-x64-Mono_Release was running with interpreter mode, too. It may running with JIT right now. @steveisok Could you please check this?)

    For JIT, it seems that the jit_time was tracking compile time for the last compiled method. In that case, it seems that it took almost the same amount of time compiling the dynamic method and whatever function before that. (or the difference is at nano second level).

    @BrzVlad Please correct me, if I were wrong.

  11. steveisok commented on Jul 15, 2021

    @steveisok
    Member

    We are using interp for iOSSimulator, but with a mode that has some JIT elements to it.

    buildArgs: -s mono+libs+host+packs+libs.tests -c $(_BuildConfig) /p:ArchiveTests=true /p:MonoForceInterpreter=true

    I have a PR up to change iOS/tvOSSimulator to be like Catalyst, which uses AOT JustInterp.

    #55577

  12. lambdageek commented on Jul 15, 2021

    @lambdageek
    Member

    Ah ok. So PlatformDetection.IsMonoInterpreter needs to be changed.

    private static bool GetIsRunningOnMonoInterpreter()
    {
    #if NETCOREAPP
    if (IsBrowser)
    return RuntimeFeature.IsDynamicCodeSupported;
    #endif
    // This is a temporary solution because mono does not support interpreter detection
    // within the runtime.
    var val = Environment.GetEnvironmentVariable("MONO_ENV_OPTIONS");
    return (val != null && val.Contains("--interpreter"));
    }

    This should probably just be

       return RuntimeFeature.IsDynamicCodeSupported && !RuntimeFeature.IsDynamicCodeCompiled;
    

    The MONO_ENV_OPTIONS check for --interpreter is wrong. (first it should probably just check for --interp, second I don't think we use the environment variable for the AppleAppBuilder - we set the execution mode in the host app directly)

  13. josalem commented on Jul 15, 2021

    @josalem
    Contributor

    Just to make sure I am grokking the full situation, so I don't duplicate efforts:

    • The iOSSimulator leg is running under the interpreter but with some jitting, hence the value we see in mono_jit_stats.jit_time.
    • GetIsRunningMonoInterpreter is currently implemented wrong causing this test to check with > instead of >=, hence the failure. <-- This requires a PR to fix the interp check
    • Steve's change will make iOSSimulator show up as an AOT mode which won't run this test at all bypassing the fialure.

    The only thing left to do besides #55577 (which should close this GH issue) is to fix PlatformDetection.IsMonoInterpreter, correct?

  14. fanyang-mono commented on Jul 15, 2021

    @fanyang-mono
    Member

    I think that mono interpreter mode should be excluded from this test as well, since the fields only matter to JIT.

  15. 2 remaining items

  16. removed
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    on Jul 15, 2021
  17. added this to the 6.0.0 milestone on Jul 15, 2021
  18. modified the milestones: 6.0.0, 7.0.0 on Aug 16, 2021
  19. added
    in-prThere is an active PR which will close this issue when it is merged
    on Aug 11, 2022
  20. self-assigned this
    on Aug 11, 2022
  21. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Aug 11, 2022
  22. ghost locked as resolved and limited conversation to collaborators on Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions