Skip to content

Avoid unnecessary allocations of jump stubs #116944

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

Merged
merged 1 commit into from
Jun 24, 2025
Merged

Avoid unnecessary allocations of jump stubs #116944

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 46 additions & 27 deletions src/coreclr/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11725,16 +11725,24 @@ void CEEJitInfo::recordRelocation(void * location,
}
else
{
//
// When m_fAllowRel32 == FALSE, the JIT will use a REL32s for direct code targets only.
// Use jump stub.
//
delta = rel32UsingJumpStub(fixupLocation, (PCODE)target, m_pMethodBeingCompiled, NULL, false /* throwOnOutOfMemoryWithinRange */);
if (delta == 0)
if (m_fJumpStubOverflow)
{
// This forces the JIT to retry the method, which allows us to reserve more space for jump stubs and have a higher chance that
// we will find space for them.
m_fJumpStubOverflow = TRUE;
// Do not attempt to allocate more jump stubs. We are going to throw away the code and retry anyway.
delta = 0;
Comment on lines +11730 to +11731
Copy link
Member

Choose a reason for hiding this comment

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

Is my understanding of what happens in the test correct?

  1. We have a first JIT attempt that sets m_fAllowRel32 = false
  2. The second JIT attempt fails to allocate a jump stub and sets m_fJumpStubOverflow = false
  3. The third try allocates a new code heap without any address range constraints and puts everything in there

Copy link
Member

Choose a reason for hiding this comment

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

Or is 1 and 2 happening during the same attempt? And the slowness of the compilation was because of needing to find 284k jump stubs, which all go into the ReserveWithinRange path that can be quite slow?

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 is 3 step process in the worst-case scenario on x64 like what's exercised by this test. It is 2 step process on arm64 (there is no allowRel32 on arm64.)

It may be possible to make it 2 step process in the worst case on x64 as well if the JIT passed more reloc details to the EE. I do not think we care enough about the extremely large methods like this one to make it better.

The second JIT attempt fails to allocate a jump stub and sets m_fJumpStubOverflow = false

Right.

The third try allocates a new code heap without any address range constraints and puts everything in there

The third allocates a code heap with enough spare space for jump stubs that we may need. Jump stubs are allocated in the same code heap as regular code and we always try to find space for jump stubs in the existing code heaps first.

}
else
{
//
// When m_fAllowRel32 == FALSE, the JIT will use a REL32s for direct code targets only.
// Use jump stub.
//
delta = rel32UsingJumpStub(fixupLocation, (PCODE)target, m_pMethodBeingCompiled, NULL, false /* throwOnOutOfMemoryWithinRange */);
if (delta == 0)
{
// This forces the JIT to retry the method, which allows us to reserve more space for jump stubs and have a higher chance that
// we will find space for them.
m_fJumpStubOverflow = TRUE;
}
}

// Keep track of conservative estimate of how much memory may be needed by jump stubs. We will use it to reserve extra memory
Expand Down Expand Up @@ -11777,24 +11785,35 @@ void CEEJitInfo::recordRelocation(void * location,
//
if (!FitsInRel28(delta))
{
// Use jump stub.
//
TADDR baseAddr = (TADDR)fixupLocation;
TADDR loAddr = baseAddr - 0x08000000; // -2^27
TADDR hiAddr = baseAddr + 0x07FFFFFF; // +2^27-1

// Check for the wrap around cases
if (loAddr > baseAddr)
loAddr = UINT64_MIN; // overflow
if (hiAddr < baseAddr)
hiAddr = UINT64_MAX; // overflow

PCODE jumpStubAddr = ExecutionManager::jumpStub(m_pMethodBeingCompiled,
(PCODE) target,
(BYTE *) loAddr,
(BYTE *) hiAddr,
NULL,
false);
TADDR jumpStubAddr;

if (m_fJumpStubOverflow)
Copy link

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

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

In the Rel28 stub allocation path, the branch for m_fJumpStubOverflow sets jumpStubAddr = 0 but does not reset delta. This could lead to an invalid relocation calculation using jumpStubAddr = 0. Consider assigning delta = 0 here or skipping the rest of the relocation logic to match the behavior in the Rel32 branch.

Copilot uses AI. Check for mistakes.
{
// Do not attempt to allocate more jump stubs. We are going to throw away the code and retry anyway.
jumpStubAddr = 0;
}
else
{

// Use jump stub.
//
TADDR baseAddr = (TADDR)fixupLocation;
TADDR loAddr = baseAddr - 0x08000000; // -2^27
TADDR hiAddr = baseAddr + 0x07FFFFFF; // +2^27-1

// Check for the wrap around cases
if (loAddr > baseAddr)
loAddr = UINT64_MIN; // overflow
if (hiAddr < baseAddr)
hiAddr = UINT64_MAX; // overflow

jumpStubAddr = ExecutionManager::jumpStub(m_pMethodBeingCompiled,
(PCODE) target,
(BYTE *) loAddr,
(BYTE *) hiAddr,
NULL,
false);
}

// Keep track of conservative estimate of how much memory may be needed by jump stubs. We will use it to reserve extra memory
// on retry to increase chances that the retry succeeds.
Expand Down
Loading