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

Profiler Interest Group - Announcements #9305

Open
noahfalk opened this issue Nov 21, 2017 · 28 comments
Open

Profiler Interest Group - Announcements #9305

noahfalk opened this issue Nov 21, 2017 · 28 comments
Labels
area-Diagnostics-coreclr design-discussion Ongoing discussion about design without consensus question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@noahfalk
Copy link
Member

I'm making this issue as an informal way to flag other issues and discussions that might be relevant to Building .NET profiling tools. This is similar to the .NET announcements repo, but given that profiling is a small crowd I didn't want to create noise for everyone or be very formal about it. Follow the issue if this is something you care about. Hopefully it works well but if not we can try something else.

Please don't do discussion directly in this issue, just links. Thanks!

@noahfalk noahfalk self-assigned this Nov 21, 2017
@noahfalk
Copy link
Member Author

Retroactive announcement - Oct 20th - I updated the profiling status page that describes our progress supporting the ICorProfiler APIs
PR: dotnet/coreclr#14644
Latest status: https://github.com/noahfalk/coreclr/blob/master/Documentation/project-docs/profiling-api-status.md

@noahfalk
Copy link
Member Author

Retroactive announcement - Oct 20th - @davmason implemented new ICorProfiler APIs (#14612, dotnet/coreclr#14643) to help profilers deal with the breaking changes that are coming with [tiered jitting] (dotnet/coreclr#12193)

@noahfalk
Copy link
Member Author

noahfalk commented Jan 5, 2018

Jan 4th 2018 - @sywhang brought up more profiling tests for ARM and OSX and we believe things are working well
PR: dotnet/coreclr#15659
Issue: #6098
Latest status: https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/profiling-api-status.md

@noahfalk
Copy link
Member Author

noahfalk commented Jan 5, 2018

Jan 4th - @sywhang fixed a bug in which the profiler invoked AppDomainCreationFinished multiple times for the same AppDomain
Issue #6804

@dotnetjt

This comment has been minimized.

@noahfalk
Copy link
Member Author

.NET Core 2.1 bug : (
We had an unintended behavior change in the ReJIT profiling API that has caused some profilers to fail. I've got a fix that restores the original behavior in the daily builds and working on getting it into 2.1 servicing releases. If you have a profiler that uses the RequestReJIT API but does not call ICorProfilerFunctionControl::SetILFunctionBody to set the IL you may have been affected. Check out dotnet/coreclr#18448 for more details and please reach out in that issue discussion if you have any questions or concerns I can help with.

@dotnetjt
Copy link

Yes, I had seen that. Fortunately, doesn't impact me. :)

@sbomer
Copy link
Member

sbomer commented Aug 15, 2018

We are adding a startup hook that could potentially be useful for profilers: dotnet/core-setup#4421. The plan is to add a DOTNET_STARTUP_HOOKS environment variable that can be used to inject managed code into the process before Main. If you have comments or suggestions, please leave them in that pull request!

@noahfalk
Copy link
Member Author

.NET Core 2.2 Preview 2 has shipped with Tiered Compilation on by default. We warned this was in progress last year above in this thread:

Retroactive announcement - Oct 20th - @davmason implemented new ICorProfiler APIs (#14612, dotnet/coreclr#14643) to help profilers deal with the breaking changes that are coming with [tiered jitting] (#12193)

If you haven't yet tested with this feature on we hope you will soon : )

@kouvel
Copy link
Member

kouvel commented Nov 13, 2018

Tiered compilation will be disabled by default for .NET Core 2.2 RTM, mainly due to https://github.com/dotnet/coreclr/issues/19752, which we plan to fix soon for 3.0

@davmason
Copy link
Member

dotnet/coreclr#22617 allows profilers to edit more types of metadata after module load (specifically adding TypeDef and MethodDef). However, adding virtual methods after module load is still unsupported.

There is a minor breaking change as part of this work. ICorProfilerInfo7::ApplyMetadata now potentially can trigger a GC, so it is now illegal to call it in situations where a GC is forbidden.

Also included in that PR is a document (located at Documentation/Profiling/Profiler Breaking Changes.md) to track breaking changes as they happen.

@noahfalk
Copy link
Member Author

If you have any questions/concerns about @davmason's changes above, as always feel free to reach out to us. Thanks!

@Maoni0
Copy link
Member

Maoni0 commented Feb 28, 2019

the existing COR_PRF_HIGH_BASIC_GC profiling is VERY heavy weight. dotnet/coreclr#22866 added a lightweight GC profiling option that just gives you

  • GC start callback
  • GC end callback
  • update generational bounds

this is something you can use on production machines. you call access this by calling ICorProfilerInfo5::SetEventMask2 and set COR_PRF_HIGH_BASIC_GC in dwEventsHigh.

@davmason
Copy link
Member

There are three recent changes to announce.

First, profiler attach/detach is now supported (#24670) leveraging the existing EventPipe work. You can find documentation on how to use it here: https://github.com/dotnet/coreclr/blob/master/Documentation/Profiling/Profiler%20Attach%20on%20CoreCLR.md. This means that the trigger process will use a completely different API, but the advantage is that it works on Linux and Windows.

Second, a new api ICorProfilerInfo10::RequestReJITWithInliners has been added (#24461) to allow profilers to ReJIT after attach. Documentation for ReJIT on attach is here: https://github.com/dotnet/coreclr/blob/master/Documentation/Profiling/ReJIT%20on%20Attach.md.

The last announcement is that ICorProfilerInfo2::DoStackSnapshot is now supported on Linux (#24968). On Linux you can call DoStackSnapshot from the same thread you are running on at any time, if you would like to get a snapshot of other threads there is now ICorProfilerInfo10::SuspendRuntime to stop all managed threads (without performing a GC) and make them safe to snapshot, and then ICorProfilerInfo10::ResumeRuntime to resume the runtime once you are finished collecting snapshots. It is not possible to snapshot an arbitrary thread on Linux. Windows support remains unchanged.

Please feel free to reach out with any questions, comments, or concerns.

@davmason
Copy link
Member

Hi Everyone,

I recently opened dotnet/coreclr#26762 to change how we deal with an attach profiler on shutdown. Currently on graceful process exit (i.e. the C# main exits normally) we issue a ICorProfilerCallback::Shutdown call and then release the profiler's COM object we hold on to, but we skip the callback and release on non-graceful shutdown.

This PR will change it so the runtime never releases the profiler's COM object, and instead lets the OS clean up during process shutdown. You will still get a ICorProfilerCallback::Shutdown call if the process is terminating gracefully.

It is my belief that this shouldn't have a negative impact on profilers, since you already have to account for the situation where the runtime doesn't free your library. Now that situation just gets a lot more common.

If you would be impacted by this change please get in touch. This issue is just for announcements so please make any comments either in the PR or on the relevant issue dotnet/coreclr#26687.

@sawilde
Copy link
Contributor

sawilde commented Sep 30, 2019

@davmason are you saying that you will always do an AddRef but never do a Release?

@davmason
Copy link
Member

@davmason are you saying that you will always do an AddRef but never do a Release?

Yes, that is the new behavior.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@davmason
Copy link
Member

Hi Everyone,

PR #1201 means that arrays are no longer special cased on class unload and profilers will be notified (via ClassUnloadStarted) when an array type is unloaded from a collectible context. Up until this point they have been purposefully excluded from ClassUnloadStarted.

I don't expect this to be a breaking change for any profilers, please get in touch if this is not the case.

@ghost
Copy link

ghost commented Apr 23, 2020

Hi Everyone,

PR #1201 means that arrays are no longer special cased on class unload and profilers will be notified (via ClassUnloadStarted) when an array type is unloaded from a collectible context. Up until this point they have been purposefully excluded from ClassUnloadStarted.

I don't expect this to be a breaking change for any profilers, please get in touch if this is not the case.

Thanks @davmason,
Currently (ever since CLRv4, including CoreCLR), neither ClassLoad* or ClassUnload* callbacks are called for arrays. Will this change ClassLoad* callbacks too?

@davmason
Copy link
Member

Currently (ever since CLRv4, including CoreCLR), neither ClassLoad* or ClassUnload* callbacks are called for arrays. Will this change ClassLoad* callbacks too?

As it stands now the class load callbacks are not called. That is because this wasn't an intentional change, but rather a side effect of removing array's TypeDesc.

Is that an issue for your profiler? It would be a fairly small change to have array types reported for class load, or to block them from being reported in class unload and go back to the old behavior.

@ghost
Copy link

ghost commented Apr 26, 2020

Currently (ever since CLRv4, including CoreCLR), neither ClassLoad* or ClassUnload* callbacks are called for arrays. Will this change ClassLoad* callbacks too?

As it stands now the class load callbacks are not called. That is because this wasn't an intentional change, but rather a side effect of removing array's TypeDesc.

Is that an issue for your profiler? It would be a fairly small change to have array types reported for class load, or to block them from being reported in class unload and go back to the old behavior.

No, this shouldn't be an issue (as we're supporting CLRv2/4 anyway). Thanks for these updates.

@davmason
Copy link
Member

Hi again everyone,

I would like to give you all a heads up about a potential breaking change.

#32283 introduced a new GC generation for the first time since very early in desktop .net. This means that there are now 5 total generations instead of the previous 4:

  • Gen 0
  • Gen 1
  • Gen 2
  • Large Object Heap
  • Pinned Object Heap

There is no specific breaking change to the ICorProfiler APIs, but I suspect that many of you will be broken because of assumptions made about the number of GC generations inside your profilers. We (the .net diagnostics team) are finding many of our tools need updating because of this change. If you are interested in the type of work we need to do, you can find that at dotnet/diagnostics#1408.

@davmason
Copy link
Member

davmason commented Nov 2, 2021

Hi everyone,

I just merged a small breaking change with #60999. There was a typo in the name of one method in the released .Net 6 corprof.idl, it does not impact any types or layouts so it will not break at runtime, but if you are building against the .net 6 headers and upgrade to this change it will cause a build break.

@gamingrobot
Copy link
Contributor

gamingrobot commented Jul 25, 2022

Wanted to post this here in case anyone else runs into it.
The changes made in #69121 will cause RequestReJITWithInliners to return CORPROF_E_REJIT_INLINING_DISABLED if a debugger is attached at startup, this only affects .NET 6.

@davmason
Copy link
Member

Hi All,

The issue reported in #76016 was found and fixed too late to be in the 7.0 GA release. It is currently scheduled to go in the first servicing update in January, PR is at #77533.

This will only impact 7.0, not previous versions. The symptoms you will see are not getting ModuleLoadFinished callbacks for dynamic modules.

If you have questions or concerns please comment in #76016 or file a new issue to not spam everyone on this thread.

@dotnet dotnet locked and limited conversation to collaborators Jun 8, 2023
@davmason
Copy link
Member

davmason commented Jun 8, 2023

Hi All,

First off some bookkeeping, I just locked the issue to make sure discussion doesn't happen here and this is limited to announcements only. I want to be clear that I welcome any and all discussion, you'll just need to file a new issue or comment on an existing one to have the discussion so this issue can be announcements only.

Next up I want to announce the new NonGC heap and related ICorProfiler APIs. For the history of the runtime all objects have been on a GC heap, but starting in .net 8 preview 5 we now keep some objects that are available at compile time on a NonGC heap. See this issue for more details: dotnet/diagnostics#4156

What this means to ICorProfiler implementations is some objects will not work with the existing GC APIs. We have added new APIs to work with NonGC objects.

interface ICorProfilerInfo14 : ICorProfilerInfo13
{
    HRESULT EnumerateNonGCObjects([out] ICorProfilerObjectEnum** ppEnum);

    HRESULT GetNonGCHeapBounds(
                    [in] ULONG cObjectRanges,
                    [out] ULONG *pcObjectRanges,
                    [out, size_is(cObjectRanges), length_is(*pcObjectRanges)] COR_PRF_NONGC_HEAP_RANGE ranges[]);
}

If you pass a NonGC object to an existing GC API you will see the error CORPROF_E_NOT_GC_OBJECT.

If possible please try out your profilers on preview 5 or later and let us know if you run in to any issues. As mentioned above feel free to reach out any time with a new issue or by commenting on dotnet/diagnostics#4156.

@dotnet dotnet unlocked this conversation Nov 24, 2023
@noahfalk
Copy link
Member Author

Hi all!

A recent PR for .NET 10 may affect profilers using Enter/Leave/Tailcall hooks.

It has always been the case that calling an ICorProfilerInfo method from within these callbacks that triggered the .NET GC to run may have corrupted memory without warning. As a profiler author the only solution was to learn which methods might do that and avoid calling them. With this PR we have reduced the set of profiler methods that trigger the GC and enabled explicit error checks for the ones that still do. After this PR is submitted calling an ICorProfilerInfo method that might trigger GC returns error code CORPROF_E_UNSUPPORTED_CALL_SEQUENCE when called from Enter/Leave/Tailcall hooks. Its possible that some profilers are currently invoking these APIs and getting lucky in not having the GC run, but now would get the error code always instead. #107152 (comment) has the list of profiler methods that might trigger GC and have this change in behavior.

If you have any questions about this or requests to make other APIs callable from Enter/Leave/Tailcall hooks we're happy to discuss. Best way is to open a new GH issue in this repo and tag @dotnet/dotnet-diag in it (don't use this issue, it is only for announcements). Thanks!

@mdh1418
Copy link
Member

mdh1418 commented Sep 20, 2024

Hello everyone!

This PR for .NET 10 affects profilers that use GetFunctionInfo2 without a COR_PRF_FRAME_INFO frameInfo obtained from a FunctionEnter2 callback.

Originally, GetFunctionInfo2 would return 0 for the ClassID if the exact match could not be determined. As brought up in #107139, there are some scenarios where a "decent" default value (e.g. System.__Canon) would suffice. With the aforementioned change, GetFunctionInfo2 will now return the MethodTable associated with the function as the ClassID rather than returning 0 if it could not be exactly determined.
Profilers should still be cautious that without passing in a COR_PRF_FRAME_INFO obtained from FunctionEnter2, the ClassID and type arguments may not be exact, as documented.

If there are any questions, please use #107139 or open a new GH issue in this repo and tag @dotnet/dotnet-diag in it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Diagnostics-coreclr design-discussion Ongoing discussion about design without consensus question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

10 participants