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

ICorProfilerCallback::ObjectsAllocatedByClass does not work anymore since dotnet 6 #63806

Closed
ogxd opened this issue Jan 14, 2022 · 12 comments · Fixed by #64284
Closed

ICorProfilerCallback::ObjectsAllocatedByClass does not work anymore since dotnet 6 #63806

ogxd opened this issue Jan 14, 2022 · 12 comments · Fixed by #64284
Assignees
Labels
area-Diagnostics-coreclr good first issue Issue should be easy to implement, good for first-time contributors
Milestone

Comments

@ogxd
Copy link
Contributor

ogxd commented Jan 14, 2022

Description

The CLR profiling API callback ICorProfilerCallback::ObjectsAllocatedByClass is not called by the CLR in dotnet 6.

Reproduction Steps

  1. Implement the ICorProfilerCallback interface
  2. Print to the console in the ObjectsAllocatedByClass callback
  3. Eventually also implement ICorProfilerCallback2 and print in the GarbageCollectionStarted callback, so that you you'll see that ObjectsAllocatedByClass is not called when GarbageCollectionStarted is.
  4. Use the COR_PRF_MONITOR_GC and COR_PRF_HIGH_MONITOR_NONE event mask 2 flags
  5. Attach to a NET 5.0 application: it works.
  6. Attach to a NET 6.0 application: it doesn't work anymore, ObjectsAllocatedByClass is not called.

Expected behavior

The ObjectsAllocatedByClass callback should be called for each garbage collection like it used to be before.

Actual behavior

The ObjectsAllocatedByClass callback is never called.

Regression?

Yes

Known Workarounds

No workaround was found

Configuration

dotnet version:

  • 6.0.101 (not working)
  • 5.0.100 (working)

OS: Windows 11 x64

Other information

Could it be due to these recent changes? @davmason
image

@dotnet-issue-labeler dotnet-issue-labeler bot added area-Diagnostics-coreclr untriaged New issue has not been triaged by the area owner labels Jan 14, 2022
@ghost
Copy link

ghost commented Jan 14, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

The CLR profiling API callback ICorProfilerCallback::ObjectsAllocatedByClass is not fired by the CLR in dotnet 6.

Reproduction Steps

  • Implement the ICorProfilerCallback interface
  • Print to the console in the ObjectsAllocatedByClass callback
  • Eventually also implement ICorProfilerCallback2 and print in the GarbageCollectionStarted callback, so that you you'll see that ObjectsAllocatedByClass is not called when GarbageCollectionStarted is.
  • Use the COR_PRF_MONITOR_GC and COR_PRF_HIGH_MONITOR_NONE event mask 2 flags
  • Attach to a NET 5.0 application: it works.
  • Attach to a NET 6.0 application: it doesn't work anymore, ObjectsAllocatedByClass is not called.

Expected behavior

The ObjectsAllocatedByClass callback should be called for each garbage collection like it used to be before.

Actual behavior

The ObjectsAllocatedByClass callback is never called.

Regression?

Yes

Known Workarounds

No workaround was found

Configuration

dotnet version:

  • 6.0.101 (not working)
  • 5.0.100 (working)

OS: Windows 11 x64

Other information

Could it be due to these recent changes? @davmason
image

Author: ogxd
Assignees: -
Labels:

area-Diagnostics-coreclr, untriaged

Milestone: -

@ogxd
Copy link
Contributor Author

ogxd commented Jan 14, 2022

So in order the check my hypothesis about the problem coming from theses changes, I installed the latest dotnet 6 SDK before this commit (6.0.100-preview.2.21155.3), and it works as expected: ObjectsAllocatedByClass is called as it should. I hope it will help finding the issue.

@davmason
Copy link
Member

Thanks for reporting this @ogxd, it is indeed caused by that change. Specifically this part:

bool AllocByClassHelper(Object * pBO, void * pv)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
MODE_ANY;
}
CONTRACTL_END;
_ASSERTE(pv != NULL);
{
BEGIN_PROFILER_CALLBACK(CORProfilerTrackAllocations());
// Pass along the call
g_profControlBlock.AllocByClass(
(ObjectID) pBO,
SafeGetClassIDFromObject(pBO),
pv);
END_PROFILER_CALLBACK();
}
return TRUE;
}

Line 1277 should be BEGIN_PROFILER_CALLBACK(CORProfilerTrackGC()) and not BEGIN_PROFILER_CALLBACK(CORProfilerTrackAllocations())

I will create a fix sometime soon, we should be able to backport this one to 6.0.

If you (or anyone else out there) are interested in contributing to the runtime, this would be a great first issue. The fix is one line, and then it's just adding a simple test to make sure ObjectsAllocByClass works.

@davmason davmason added good first issue Issue should be easy to implement, good for first-time contributors and removed untriaged New issue has not been triaged by the area owner labels Jan 14, 2022
@davmason davmason self-assigned this Jan 14, 2022
@davmason
Copy link
Member

Oh, and it should be possible to work around this by enabling COR_PRF_MONITOR_OBJECT_ALLOCATED, although that is a very expensive thing to enable so I wouldn't suggest enabling that workaround in any production scenario

@ogxd
Copy link
Contributor Author

ogxd commented Jan 14, 2022

Hi @davmason, thanks for the quick reply! A backport fix would be amazing :)
Unfortunately, I can't use COR_PRF_MONITOR_OBJECT_ALLOCATED since the profiler I use is attached to an already running process (using InitializeForAttach(...)), and, from what I understood, it is only possible to use this flag for profilers that are initialized when the profiled app starts (using Initialize(...))

I'll try submitting a fix 👍

@davmason
Copy link
Member

For the workaround, it might just work if you set COR_PRF_MONITOR_OBJECT_ALLOCATED even on attach. You won't get the object allocated callbacks, since you need to set those at startup like you mention, but I don't think we prevent you from setting it later in the process.

I would like to see a test if you submit a fix, it hopefully is relatively straightforward but if you get blocked I'm happy to help get you going again.

You need to add a managed testcase, like inlining.csproj/inlining.cs in https://github.com/dotnet/runtime/tree/main/src/tests/profiler/unittest, and then a corresponding native profiler like the one in https://github.com/dotnet/runtime/tree/main/src/tests/profiler/native/inlining.

The managed app should allocate some classes, and then the native profiler should check that the classes it expected to see were allocated and write "PROFILER TEST PASSES" if everything is right.

@davmason
Copy link
Member

Oh, nevermind about the workaround. I forgot we whitelist the allowable after attach flags. It won't work

@ogxd
Copy link
Contributor Author

ogxd commented Jan 19, 2022

Thanks for the review and the merge @davmason! Shall I make another PR for an eventual backport to 6.0.0 or do you prefer to handle this on your side?

@davmason
Copy link
Member

We have a bot that will do the actual work of opening a PR, but then it has to go through an approval process that I have no control over. I hope it gets through since we have a customer (you) asking for it and it is small, but can't promise anything.

@davmason
Copy link
Member

/backport to release/6.0

@davmason
Copy link
Member

Don't know why the backport bot isn't working, it looks like I'll have to create it manually

@davmason
Copy link
Member

davmason commented Mar 2, 2022

Closing since it is backported to 6 and in main

@davmason davmason closed this as completed Mar 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Diagnostics-coreclr good first issue Issue should be easy to implement, good for first-time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants