Implement ICorProfilerInfo14::EnumerateNonGCObjects #85100
Conversation
|
Tagging subscribers to this area: @tommcdon Issue DetailsContributes to dotnet/diagnostics#4156 Will add tests once #85017 is merged (to the same project).
|
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -7586,6 +7586,39 @@ HRESULT ProfToEEInterfaceImpl::GetObjectIDFromHandle( | |||
| return S_OK; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
You'll also need to add ICorProfilerInfo14 to ProfToEEInterfaceImpl::QueryInterface
ghost
added
the
needs-author-action
|
Besides adding the QI it looks good to me |
| } | ||
| CONTRACTL_END; | ||
|
|
||
| GCX_COOP(); |
There was a problem hiding this comment.
What portion of the implementation requires us to switch to coop mode? This might make the API challenging to use without a deadlock. The primary use I am anticipating from profiler vendors is that they would enumerate both the GC and non-GC heaps during a GC. This means they would invoke this API inside one of the callbacks that GC is starting or finishing and at that point the runtime would be suspended. I'm guessing the transition to COOP mode would block until the EE resumes, the EE won't resume until the profiler GC callback returns, and the GC callback won't return until this enumeration completes.
There was a problem hiding this comment.
It's not needed indeed, removed
| // Let's make sure we got the same number of objects as we got from the callback | ||
| // by testing the EnumerateNonGCObjects API. | ||
| ICorProfilerObjectEnum* pEnum = NULL; | ||
| HRESULT hr = pCorProfilerInfo->EnumerateNonGCObjects(&pEnum); |
There was a problem hiding this comment.
Can you modify the test case to enable the profiler COR_PRF_MONITOR_GC events and then try doing this enumeration inside the GarbageCollectionFinished callback? I think that is one of the more likely places a profiler might want to enumerate the non-GC heap. I'm suspicious the current implementation may deadlock but even if all works well it will be good for us to confirm that scenario doesn't regress.
ghost
removed
the
needs-author-action
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
CI failures are known timeouts (#76454) |
Contributes to dotnet/diagnostics#4156