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

Initial hot cold splitting support for crossgen2/VM #1900

Conversation

cshung
Copy link
Member

@cshung cshung commented Jun 18, 2022

This PR marks the start of the work to get crossgen2 to support hot/cold splitting.

Here are the changes:

  1. Added a section named Scratch, to be described in (13) below for more detail.
  2. Create the _methodColdCode from _coldCode and _coldCodeRelocs during PublishCode.
  3. Null _methodColdCode and empty _coldCodeRelocs during CompileMethodCleanup.
  4. Create _methodColdCode during allocMem.
  5. Change reserveUnwindInfo to ignore unwind reservation for coldCode.
  6. Change allocUnwindInfo to ignore unwindInfo allocation for 0 bytes.
  7. Change findKnownBlocks to return _coldCodeReloc when BlockType.ColdCode is asked for.
  8. Change recordRelocation to set relocTarget to _methodColdCodeNode when BlockType.ColdCode is asked for.
  9. Implement MethodColdCodeNode
  • Storing the cold code.
  • Make sure sorting will place cold code blocks after hot code blocks.
  • Make sure cold code blocks are ordered the same as the hot code blocks.
  1. Change MethodGCInfoNode to
  • Emit the chained unwind info for the cold code block, after the hot code GCInfo, so the hot and cold code unwind info are currently mixed.
  • Changed CalculateFuncletOffsets so that the offset for the cold code chained unwind info is included if it exists.
  1. Changed MethodWithGCInfo to:
  • Have a reference to the MethodColdCodeNode.
  • Changed GetFixupBlob to include relocation for the cold code.
  1. Changed RuntimeFunctionTableNode to:
  • Emit the RuntimeFunction entries for the cold code block after the hot code blocks.
  • Prepare the cold to hot code mapping for ScratchNode.
  1. Implement ScratchNode - the session is simply an array of cold code runtime function index to hot code runtime function index, so all even entries (e.g. 0, 2, 4) are cold code runtime function index and the corresponding odd indexes (e.g. 1, 3, 5) are the corresponding runtime function index for the corresponding hot code. The array is sorted by the cold code indexes, by (9), the hot code indexes should be sorted as well.
  2. Implemented the --hot-cold-splitting command-line option, and turn on the CORJIT_FLAG_PROCSPLIT flag as needed.
  3. Implemented the various code manager contracts:
  • JitCodeToMethodInfo already gave us a runtime function index, we do not know if it is hot or cold, so we use the scratch table to find it. If we got an even index, that would be cold code and therefore we map it back to the hot one.
  • Change GetFuncletStartOffsets to return the cold code block as well - technically the cold block is not a funclet, but it appears that the debugger might want it.
  • Override IsFunclet so that it avoids returning true for the cold code block, by searching the scratch table.
  • Change JitTokenToMethodRegionInfo so that it will find and return the cold code block - again, by search the scratch table since we already have the hot part runtimeFunction.
  • Change DebugDebugger so that it uses EECodeInfo to compute relative offset instead of a simple subtraction, that will allow us to account for the cold code.

This work is far from complete - here is a list of issues filed for the project - these are known issues for now:

@amanasifkhalid, @BruceForstall

@dotnet/crossgen-contrib

@cshung cshung force-pushed the public/bootstrapping-hot-cold-splitting branch 3 times, most recently from 5280674 to 4e17526 Compare June 18, 2022 18:52
@cshung cshung force-pushed the public/bootstrapping-hot-cold-splitting branch from 4e17526 to 3ce1cd5 Compare June 18, 2022 19:06
Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

This is a good start. Let's get it merged so we can enable collaboration.

@BruceForstall
Copy link
Member

Added #1913

@BruceForstall
Copy link
Member

Have you followed all the instructions on https://github.com/dotnet/runtimelab/blob/docs/CreateAnExperiment.md to make this a "proper" runtimelab feature branch?

@cshung cshung merged commit 658f03b into dotnet:feature/hot-cold-splitting Jun 21, 2022
@cshung
Copy link
Member Author

cshung commented Jun 21, 2022

I merged the PR for now, feel free to work on top of it.
In the meantime, I will follow up with @hoyosjs to make sure the branch is set up correctly.
In case it isn't, there might be some churns to the actual commits, but it shouldn't impact the code at all.

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

I have added some feedback that should be ultimately addressed by the final implementor of hot-cold code splitting in Crossgen2 (Eugenio); in general the code changes seem reasonable to me, thanks Andrew for providing our intern with such a great head start!

@hoyosjs
Copy link
Member

hoyosjs commented Jun 21, 2022

I merged the PR for now, feel free to work on top of it. In the meantime, I will follow up with @hoyosjs to make sure the branch is set up correctly. In case it isn't, there might be some churns to the actual commits, but it shouldn't impact the code at all.

Looked at all things I can think of - looks fine to me

@MichalStrehovsky
Copy link
Member

I merged the PR for now, feel free to work on top of it. In the meantime, I will follow up with @hoyosjs to make sure the branch is set up correctly. In case it isn't, there might be some churns to the actual commits, but it shouldn't impact the code at all.

Looked at all things I can think of - looks fine to me

The instructions also suggest deleting .github/CODEOWNERS. It's the reason why I was pinged for review (I don't mind that part). But it will also cause a lot of people to be pinged for review if you ever do a merge pull request from runtime main branch (because those touch a lot of files). Deleting it will avoid bothering those people. Doesn't apply if you don't intend to merge from main.

@cshung cshung deleted the public/bootstrapping-hot-cold-splitting branch June 22, 2022 21:02
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.

5 participants