Skip to content

Adding StopProfile method to System.Runtime.ProfileOptimization class (MulticoreJit). #33426

Closed

Description

@0xfk0

Issue description

Currently dotnet runtime (CoreCLR) has System.Runtime.ProfileOptimization class which is intendent to improve application startup performance (decrease startup time). ProfileOptimization class in just an interface to "MulticoreJIT" feature of dotnet runtime, which is implemented in scope of CoreCLR project.

The principle of operation of "MulticoreJIT" feature is to record "profile" -- which methods of which classes needs to be compiled during program startup, and later, when program is started second time, "replay" this profile: load necessary classes, and compile required methods in advance, in parallel thread.

Currently ProfileOptimization class implements only two methods:

  • void SetProfileRoot (string directoryPath);
  • void StartProfile (string profile).

First one sets directory, where profile should be stored. Second one specifies file name for the profile and starts recording and replaying the profile (if it was previously recorded).

It is obviously, that with such interface, consisting only of two functions, it's not clear when profiling should be stopped. Documentation says that:

The code that performs the recording tracks the rate at which methods are compiled, including methods that are compiled on both the background thread and on the main thread. When the rate falls below a predetermined level, recording stops. Recording also stops if the rate of JIT compilation exceeds a predetermined upper limit.

So where is no clear point at which profiling to be stopped before termination of the program. Only when program terminates the profile is guaranteed to be finished.

This leads us to two issues:

  1. some applications might never be stopped, for example, "server" applications, or long lived UI applications which is "minimized" to tray, but not closed -- for such applications profile might never be written to file;

  2. as there is no clear condition when profiling must be stopped, this might lead to recording of excess amount of profiling data and consequently to slowing down startup time (because of class loading and compiling the methods which, actually, not required for fast startup).

For last case I want to give an example: imagine computer game, which can be quickly started and then displays title screen, at which user can select game's level and press "start" button. After "start" button pressed, selected level is loaded (which requires loading lot of classes and compilation lot of methods). We want to stop recording profile just after title screen is shown, and we didn't want to record anything related to selected game level: because next time another level might be selected, and loading classes and compiling methods for level selected at previous time is waste of time.

Another example: GUI application, where some heavyweight function (which requires loading of many classes and compilation lot of methors) is started by user command. If the profiling isn't stopped at right time, the profile may include unwanted classes and methods.

To fight first issue (profile never written due to the fact, that dotnet runtime instance is never terminated) CoreCLR currently implements timeout, which might be tuned via environment variable COMPlus_MultiCoreJitProfileWriteDelay (default is 12 seconds). Unfortunately timeout
works only on Windows platform, but not on Unix (no code for this function in MulticoreJit component).

But solution with timeout not solves second issue: user might start gameplay within 12 second interval. And with any interval we always will be in question -- this inteval is too short, to record all required methods, or too long, to capture lot of unwanted methods.

Second issue and both examples listed above is the main reason for requesting extensions of the API: timeout isn't reliable way to control when profiling should be finished. We want to stop profiling at some exact state of the program, typically, when GUI application finished redrawing of main menu, title screen, etc...

Proposed solution

The proposition is to add into System.Runtime.ProfileOptimization class class a method with the following name and signature:

        public static void StopProfile();

This method should stop collecting the profile and save collected profile in the file pointed by StartProfile function (which should be previously called). By using this method, the application's creator might control exact period of time, at which profile is collected.

In most cases, for GUI applications, StartProfile is called after program start, and StopProfile should be called after displaying main window or initial screen of the application. For non GUI, "server" applications, StopProfile might be called after application entered in "main cycle" (accepting network connections, for example, etc...)

If StopProfile called when no StartProfile called before, or called twice, the function should do nothing.

If StopProfile isn't called at all, then "MulticoreJIT" feature's logic shouldn't be changed and profiling might be stopped by timeout, or at the program termination time.

Pull Request

The following pull request implements proposed changes: #33252

Also changes in documentation required, which should note existence of new function.

Activity

  1. added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    untriagedNew issue has not been triaged by the area owner
    on Mar 10, 2020
  2. 0xfk0 commented on Mar 10, 2020

    @0xfk0
    ContributorAuthor
  3. 0xfk0 commented on Mar 10, 2020

    @0xfk0
    ContributorAuthor
  4. jkotas commented on Mar 10, 2020

    @jkotas
    Member

    The proposition is to add into System.Runtime.ProfileOptimization class

    Your PR is adding the method to both this class, and also to AssemblyLoadContext. Should the proposal contain both these methods?

  5. added
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Mar 10, 2020
  6. jkotas commented on Mar 10, 2020

    @jkotas
    Member

    @noahfalk @kouvel Any concerns about adding this method?

  7. 0xfk0 commented on Mar 10, 2020

    @0xfk0
    ContributorAuthor

    Call to StopProfile made in same way, as call to StartProfile, which is goes via AssemblyLoadContext class. Why this done so for StartProfile I don't know. I think, it meant, that other contexts than AssemblyLoadContext.Default might be used, but practically only one default context exists now. I think you should know better.

  8. jkotas commented on Mar 10, 2020

    @jkotas
    Member

    It seems that StartProfile(null) does what you need already:

    StopProfile(false);

    Is that correct? If it is the case, we may just need to fix the documentation.

  9. 0xfk0 commented on Mar 11, 2020

    @0xfk0
    ContributorAuthor

    Yes, we can use StartProfile(null) to stop profiling. So no code changes required, and only documentation change required at your side.

    How about if somebody changes logic of MulticoreJitManager::StartProfile function in future? I think such behaviour (stopping profiling when StartProfile(null) is called) should be documented in code.

  10. jkotas commented on Mar 11, 2020

    @jkotas
    Member

    code changes required,

    • The nullability annotations for StartProfile should be change. It should be string?.
    • Add a test that passes null to start profile. We should at least have a tests that verify the API does not crash when you pass in null.
    • More code comments would be useful too as you have pointed out

    The API documentation should be updated in https://github.com/dotnet/dotnet-api-docs

    @kfrolov Would you like to submit PR to fix this?

  11. added this to the 5.0 milestone on Mar 11, 2020
  12. removed
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Mar 11, 2020
  13. jkotas commented on Mar 23, 2020

    @jkotas
    Member
  14. added a commit that references this issue on Mar 24, 2020
    4464edd
  15. 0xfk0 commented on Mar 24, 2020

    @0xfk0
    ContributorAuthor

    Looks like changes in documentation not needed. I have found, that currently existing documentation currently contains following phrase:

    Calling this method stops any previous recording in progress. Calling this method with a null profile file name stops any recording in progress and does not start a new recording

    So, we only need to prepare another PR to add nullability annotations, modify test and add comments.

  16. jkotas commented on Mar 24, 2020

    @jkotas
    Member

    I did not see any response from you, so I just took care of it. This is all done now.

  17. ghost locked as resolved and limited conversation to collaborators on Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions