Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Preallocate jump stubs for dynamic methods #9883

Merged
merged 4 commits into from
Mar 2, 2017

Conversation

kouvel
Copy link
Member

@kouvel kouvel commented Mar 1, 2017

  • This eliminates the possibility of running into an out-of-memory situation after compiling the method
  • The temporary entry points block containing FixupPrecodes is extended for dynamic methods to include sufficient space for jump stubs
  • When the target is too far for the FixupPrecode to encode a short relative jump, it instead does a short relative call or jump to the corresponding jump stub, which does an absolute jump to the target

- This eliminates the possibility of running into an out-of-memory situation after compiling the method
- The temporary entry points block containing FixupPrecodes is extended for dynamic methods to include sufficient space for jump stubs
- When the target is too far for the FixupPrecode to encode a short jump, it instead does a short call or jump to the corresponding jump stub, which does a far jump to the target
@kouvel
Copy link
Member Author

kouvel commented Mar 1, 2017

@jkotas @BruceForstall

kouvel added a commit to kouvel/corefx that referenced this pull request Mar 1, 2017
kouvel added a commit to kouvel/corefx that referenced this pull request Mar 1, 2017
kouvel added a commit to kouvel/corefx that referenced this pull request Mar 1, 2017
kouvel added a commit to kouvel/corefx that referenced this pull request Mar 1, 2017
{
CONTRACTL
{
THROWS; // emptBackToBackJump may throw (see emitJump)
Copy link
Member

Choose a reason for hiding this comment

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

emptBackToBackJump [](start = 19, length = 18)

nit: typo: emptBackToBackJump

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@BruceForstall
Copy link
Member

LGTM

@BruceForstall
Copy link
Member

Did you compute/measure the additional size impact of this?

Does NGEN store FixupPrecode? What is the size increase, say, of System.Private.CoreLib.ni.dll? Does nidump still work? Did you test on ngen'ed images?

@kouvel
Copy link
Member Author

kouvel commented Mar 1, 2017

Since the change only applies to dynamically created methods, my understanding is that it should have no impact on ngen/crossgen. I'll check what you suggested.

@kouvel
Copy link
Member Author

kouvel commented Mar 1, 2017

Regarding runtime memory impact, since we were allocating an additional 8 bytes per FixupPrecode, which wasn't being used, after this change the additional impact is 4 bytes per FixupPrecode for a dynamic method (12 bytes per jump stub).

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Looks great!

@kouvel kouvel merged commit 4bafc10 into dotnet:master Mar 2, 2017
@kouvel kouvel deleted the DynamicMethodJumpStub branch March 2, 2017 04:52
@kouvel
Copy link
Member Author

kouvel commented Mar 2, 2017

Does NGEN store FixupPrecode?

Looks like it, but it shouldn't be saving any FixupPrecodes for dynamic methods

What is the size increase, say, of System.Private.CoreLib.ni.dll?

No change

Does nidump still work?

It should, but while I found the code for it I didn't find info on how to initiate a dump. Could you please point me to some info on this?

Did you test on ngen'ed images?

No, I can maybe diff the nidump before and after and see if there is anything to test

@BruceForstall
Copy link
Member

Given that ngen images shouldn't include FixupPrecode, I wouldn't worry about it.

I normally use "nidump /?" or similar to remind myself of the commands. Maybe it's not built in the open? I haven't looked.

@jkotas
Copy link
Member

jkotas commented Mar 2, 2017

Fragile ngen images do include FixupPrecode - a lot of them.

However, this change have not changed anything near NGen or nidump.

jorive pushed a commit to guhuro/coreclr that referenced this pull request May 4, 2017
Preallocate jump stubs for dynamic methods

- This eliminates the possibility of running into an out-of-memory situation after compiling the method
- The temporary entry points block containing FixupPrecodes is extended for dynamic methods to include sufficient space for jump stubs
- When the target is too far for the FixupPrecode to encode a short relative jump, it instead does a short relative call or jump to the corresponding jump stub, which does an absolute jump to the target
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Preallocate jump stubs for dynamic methods

- This eliminates the possibility of running into an out-of-memory situation after compiling the method
- The temporary entry points block containing FixupPrecodes is extended for dynamic methods to include sufficient space for jump stubs
- When the target is too far for the FixupPrecode to encode a short relative jump, it instead does a short relative call or jump to the corresponding jump stub, which does an absolute jump to the target

Commit migrated from dotnet/coreclr@4bafc10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants