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

Document the JIT bootstrap process #21003

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Jan 23, 2025

Closes #12506

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 23, 2025

@a7ehuo could you please review?

Copy link
Contributor

@a7ehuo a7ehuo left a comment

Choose a reason for hiding this comment

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

Thank you very much Irwin for documenting the JIT bootstrap process! I just have a few very minor comments

doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 24, 2025

@a7ehuo good for another review

Copy link
Contributor

@a7ehuo a7ehuo left a comment

Choose a reason for hiding this comment

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

Thank you for the update! I just have two minor comments

doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
Copy link
Contributor

@a7ehuo a7ehuo left a comment

Choose a reason for hiding this comment

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

Thank you for the update! LGTM

@dsouzai
Copy link
Contributor Author

dsouzai commented Jan 24, 2025

@0xdaryl could you review/merge?

@dsouzai
Copy link
Contributor Author

dsouzai commented Feb 5, 2025

@0xdaryl review reminder. If you're not able to, perhaps @mpirvu do you mind reviewing/merging?

@0xdaryl
Copy link
Contributor

0xdaryl commented Feb 5, 2025

Sorry, this slipped off my radar. I'll review, but I would still appreciate @mpirvu's eyes on this as well.

Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Personally I would also want to see a comment on when the compilation threads are stopped and also when does "LIBRARIES_ONUNLOAD" get executed. In all my tests I see only "JVM_EXIT_STAGE"; "LIBRARIES_ONUNLOAD" frees some extra data structure, so would be nice to know how to test that code path.

doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Outdated Show resolved Hide resolved
doc/compiler/runtime/Bootstrap.md Show resolved Hide resolved
@dsouzai
Copy link
Contributor Author

dsouzai commented Feb 5, 2025

In all my tests I see only "JVM_EXIT_STAGE"; "LIBRARIES_ONUNLOAD" frees some extra data structure, so would be nice to know how to test that code path.

It's quite a roundabout mechanism for LIBRARIES_ONUNLOAD. It looks like LIBRARIES_ONUNLOAD is invoked via runShutdownStage, but this happens really early in protectedInitializeJavaVM. It only invokes libraries that have set the FORCE_UNLOAD flag. The JIT does set this under the following condition when it is hooked in the DLL_LOAD_TABLE_FINALIZED stage.

         static bool TR_DisableFullSpeedDebug = feGetEnv2("TR_DisableFullSpeedDebug", (void *)vm)?1:0;
         if (!fullSpeedDebugSet || TR_DisableFullSpeedDebug)
            {
            loadInfo->loadFlags |= FORCE_UNLOAD;
            break;
            }

It seems that if we use the env var to disable FSD, we tell the VM to just unload the JIT.

[ci-skip]

Signed-off-by: Irwin D'Souza <[email protected]>
@dsouzai
Copy link
Contributor Author

dsouzai commented Feb 5, 2025

@mpirvu @0xdaryl comments addressed.

@0xdaryl
Copy link
Contributor

0xdaryl commented Feb 5, 2025

Docs only change. No need for CI.

@0xdaryl 0xdaryl merged commit 6410fb4 into eclipse-openj9:master Feb 5, 2025
2 checks passed
@0xdaryl 0xdaryl self-assigned this Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document compiler bootstrap process
4 participants