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

[Performance] LoaderUtil.normalizePath on windows takes a significant amount of launch time #970

Closed
MCRcortex opened this issue Aug 30, 2024 · 2 comments · Fixed by #971
Closed
Assignees

Comments

@MCRcortex
Copy link

On windows LoaderUtil.normalizePath takes a significant amount of time during minecraft launch, (8% overall, 17% just on the render thread)

This is because path.toRealPath(); on windows is a very strangled mess of calls, invoking many native methods dispatches, buffers , reconstructing and reparsing the entire file path (multiple times). Point is, it is a slow method.

This is not an issue generally, however KnotClassDelegate.getCodeSource calls it every time.
This results in it taking more time to resolve the file path than it does to load and define the class.

Attached are profile results of a minecraft launch to the main menu.

All threads: image
Render thread only:
image

Patching KnotClassDelegate.getCodeSource to not normalize the file path (i.e. just return the non-normalized path) there was a noticeable decrease in launch time.

@MCRcortex
Copy link
Author

One potential solution is to test the nonnormalized path inside getMetadata, if it is not in the cache, normalize it, check against cache (generate if miss)
It does mean that there are potentially multiple paths mapping to the same Metadata, however that should not cause any issues.

@sfPlayer1
Copy link
Contributor

Thanks for the great report, I prepared a PR that should eliminate most of the redundant calls. It'd be great if you could give it a try

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 a pull request may close this issue.

2 participants