Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

With this, PerfView will be able to create native AOT gcdumps. It activates the same codepaths we had for Project N, so it will also properly decode type names.

The changes to DotNetHeapDumpGraphReader match dotnet/diagnostics#5506.

With this, PerfView will be able to create native AOT gcdumps. It activates the same codepaths we had for Project N, so it will also properly decode type names.

The changes to DotNetHeapDumpGraphReader match dotnet/diagnostics#5506.
Copy link
Member

@brianrob brianrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this @MichalStrehovsky. A few questions.

Debug.Assert(typeName.Length == 0);
Debug.Assert((typeData.Flags & TypeFlags.ModuleBaseAddress) != 0);
var moduleBaseAddress = typeData.TypeID - (ulong)typeData.TypeNameID; // Tricky way of getting the image base.
ulong moduleBaseAddress = typeData.ModuleID;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you've done here makes sense to me, but I am not sure why we didn't just use ModuleID for ProjectN. Do you know if this will work for ProjectN as well? Perhaps this was just an oversight?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will work for Project N too. Project N started populating this with the OS module handle as part of TFS changeset 1217253 ("ETW changes to unblock Project N GC Heap Dump feature inside PerfView."). I assume PerfView didn't work before that change, so the first builds that have this changeset should have the correct thing in the ModuleID.

{
GCBulkTypeValues typeData = data.Values(i);
var typeName = typeData.TypeName;
if (IsProjectN)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you're removing usages of IsProjectN where it makes sense. Canyou confirm that the instance on line 382 should remain? If not, I think we are safe to remove IsProjectN entirely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I believe that should remain, see dotnet/diagnostics#5506 (comment).


if (!hasDotNet && !hasJScript && !hasMrt && !hasCoreClr)
{
m_log.WriteLine("No supported runtime type detected, going to assume native AOT.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there another way to detect that we're dealing with Native AOT? I suppose that trying this as a last resort is OK if necessary, since we can currently detect the other cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot detect it based on loaded modules.

The only externally visible marker we have is DotnetRuntimeDebugHeader exported symbol that is used by the debugger extension. I wasn't sure if we wanted to go as far as reading process memory and parsing the exports directory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks. I'd say we can defer on that for now. If we end up with another fully native scenario, we can cross that bridge then.

@brianrob brianrob merged commit 372d5a6 into microsoft:main Jun 20, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants