Skip to content

Fixup CodeFragmentHeap block allocation reporting #127298

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.

Sign up for GitHub

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

Closed
hoyosjs wants to merge 2 commits into from
Closed

Fixup CodeFragmentHeap block allocation reporting #127298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ba8003
Report code size after segments have been commited
hoyosjs Apr 22, 2026
f6bf674
Get unique name for stub fix
hoyosjs Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/coreclr/vm/codeman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2213,7 +2213,6 @@ TaggedMemAllocPtr CodeFragmentHeap::RealAllocAlignedMem(size_t dwRequestedSize
if (dwSize < SMALL_BLOCK_THRESHOLD)
dwSize = 4 * SMALL_BLOCK_THRESHOLD;
pMem = ExecutionManager::GetEEJitManager()->AllocCodeFragmentBlock(dwSize, dwAlignment, m_pAllocator, m_kind);
ReportStubBlock(pMem, dwSize, m_kind);
}

SIZE_T dwExtra = (BYTE *)ALIGN_UP(pMem, dwAlignment) - (BYTE *)pMem;
Expand All @@ -2227,6 +2226,8 @@ TaggedMemAllocPtr CodeFragmentHeap::RealAllocAlignedMem(size_t dwRequestedSize
dwSize -= dwRemaining;
}

ReportStubBlock(pMem, dwSize, m_kind);
Copy link
Copy Markdown
Member

@tommcdon tommcdon Apr 23, 2026

Choose a reason for hiding this comment

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

Could we end up double reporting blocks if we don't call AllocCodeFragmentBlock?

Copy link
Copy Markdown
Member Author

@hoyosjs hoyosjs Apr 23, 2026

Choose a reason for hiding this comment

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

you can end up grabbing it from the best fit path - that but since that got aligned down and returned during the if block right above this, that would cound as a separate block and should be reported separately

Copy link
Copy Markdown
Member

@jkotas jkotas May 21, 2026

Choose a reason for hiding this comment

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

This logging is going to be duplicating the PerfMapStubType::IndividualWithinBlock logging done by the callers now. I do not think that it is the intention here. I think the intention here is to report the whole fragment and let callers report the fine-grained details for the chunks of the fragment.

Copy link
Copy Markdown
Member Author

@hoyosjs hoyosjs May 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

If the intention is to have both reports and one is overlapped and then samples are corrected, then fec5107 is enough. That's the behavior of perf - so all good.


TaggedMemAllocPtr tmap;
tmap.m_pMem = pMem;
tmap.m_dwRequestedSize = dwSize;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/perfmap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ bool PerfMap::s_ShowOptimizationTiers = false;
bool PerfMap::s_GroupStubsOfSameType = false;
bool PerfMap::s_IndividualAllocationStubReporting = false;

unsigned PerfMap::s_StubsMapped = 0;
volatile LONG PerfMap::s_StubsMapped = 0;
CrstStatic PerfMap::s_csPerfMap;

bool PerfMapLowGranularityStubs()
Expand Down Expand Up @@ -437,7 +437,7 @@ void PerfMap::LogStubs(const char* stubType, const char* stubOwner, PCODE pCode,
}
else
{
name.Printf("stub<%d> %s<%s>", ++(s_StubsMapped), stubType, stubOwner);
name.Printf("stub<%d> %s<%s>", InterlockedIncrement(&s_StubsMapped), stubType, stubOwner);
}
Comment thread
hoyosjs marked this conversation as resolved.
SString line;
line.Printf(FMT_CODE_ADDR " %x %s\n", pCode, codeSize, name.GetUTF8());
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/perfmap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class PerfMap
static bool s_GroupStubsOfSameType;
static bool s_IndividualAllocationStubReporting;

// Set to true if an error is encountered when writing to the file.
static unsigned s_StubsMapped;
// Counter for generating unique stub names when s_GroupStubsOfSameType is false.
static volatile LONG s_StubsMapped;

static CrstStatic s_csPerfMap;

Expand Down
Loading