Skip to content

Optimise path finding to remove the last exists test#45

Merged
marchermans merged 2 commits intoMcModLauncher:mainfrom
Matyrobbrt:optimise-path-finding
Nov 9, 2023
Merged

Optimise path finding to remove the last exists test#45
marchermans merged 2 commits intoMcModLauncher:mainfrom
Matyrobbrt:optimise-path-finding

Conversation

@Matyrobbrt
Copy link
Copy Markdown
Member

UnionFileSystem#findFirstFiltered will currently check if a file exists for every merged path. This is a bit inefficient since this causes possibly unneeded existence checks, that can in some cases be heavy (for example, native checks on Windows for files on the C drive).
This PR makes findFirstFiltered not check for the existence of the last path, and simply return it if all previous paths did not exist. As in all cases where the method is used, if the optional is empty a NoSuchFileException will be raised (intentionally), this is a safe change to make.

Benchmark; the declines can be safely considered error margin.

@Matyrobbrt Matyrobbrt added the enhancement New feature or request label Nov 6, 2023
@Matyrobbrt
Copy link
Copy Markdown
Member Author

Matyrobbrt commented Nov 6, 2023

As a tangent, the != notExistingPath checks also seem to be completely useless, and should be removed if they actually serve no purpose. The exact instance of that field seems not to be returned anywhere.

Comment thread src/main/java/cpw/mods/niofs/union/UnionFileSystem.java Outdated
Copy link
Copy Markdown
Contributor

@Technici4n Technici4n left a comment

Choose a reason for hiding this comment

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

:shipit:

@marchermans marchermans merged commit 482f95d into McModLauncher:main Nov 9, 2023
@Matyrobbrt Matyrobbrt deleted the optimise-path-finding branch September 4, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants