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

Using Java 11's readString and writeString on modern JREs #233

Closed
wants to merge 1 commit into from

Conversation

mkarg
Copy link
Collaborator

@mkarg mkarg commented Jan 15, 2023

As the latest Java release was 19 (and 20 will be here in few weeks), it makes sense to allow users of those JREs to take advantage of their superior APIs. In this PR, I am proposing to use the single-line Files.readString and Files.writeString methods on all modern JREs (11+) instead of dealing with interim custom byte-to-String operations needed since JRE 7 through 10. For the sake of this, I have adopted the multi-release pattern to FileUtils in the same way we already applied it years ago to BaseIOUtils. As such, JRE 11 through 21 (and further) will be as fast as possible when reading / writing text files.

@mkarg
Copy link
Collaborator Author

mkarg commented Jan 15, 2023

As the commits list shows I had to fix two bugs in the test code to pass on Windows. These are actually not dependend of the actual changes done in this PR, but had not been detected prior without those changes. Hence I do not provide them in distinct PRs, but keep them as part of this one.

@mkarg
Copy link
Collaborator Author

mkarg commented Jan 18, 2023

@michael-o Kindly requesting your review. :-)

@olamy
Copy link
Member

olamy commented Jan 18, 2023

TBH I would make just p-u jdk11 mandatory to get rid of this over complicated multi release jars....

@mkarg
Copy link
Collaborator Author

mkarg commented Jan 19, 2023

@olamy I am +1 with only supporting JDK11 from now on, but is it realisitic? Also it wouldn't solve the core problem: As soon as JDK 21 comes up with even easier and even faster APIs, I certainly want to use it -- and we are back at multi-release JARs.

Having said that, anybody willing to incorporate my PR?

@mkarg
Copy link
Collaborator Author

mkarg commented Jan 21, 2023

I am a bit disappointed that this PR simply is ignored mostly. Using Java 11's API provides superior performance, so is really beneficial. Why not simply merging this PR (it does not harm) to allow me to benefit from the changes, but go on with @olamy's proposal after that? Any comment is better than being silent! Thanks! :-)

@michael-o
Copy link
Member

I am a bit disappointed that this PR simply is ignored mostly. Using Java 11's API provides superior performance, so is really beneficial. Why not simply merging this PR (it does not harm) to allow me to benefit from the changes, but go on with @olamy's proposal after that? Any comment is better than being silent! Thanks! :-)

What superior performance?

@mkarg
Copy link
Collaborator Author

mkarg commented Jan 21, 2023

What superior performance?

Thank you for continuing the discussion about this PR. :-)

JDK 11's Files.readString allows the JRE to perform internal tricks like prevention of unnecessary data duplication. It does that by calling OpenJDK-specific APIs internally. Such optimizations are impossible for custom code, as these internal APIs are not public. Hence, JRE 8's API potentially creates comparably higher memory use and higher GC pressure, both resulting in worse performance. While this might not be an actual problem for every application (possibly it might not even be a problem for Plexus Utils), it OTOH neither harms any application to use JRE 11's API by default, whenever running on JRE 11+, hence this API should be used if any possible (at least this is the general idea why we(*) create new APIs in OpenJDK frequently). There is no good reason to not use the APIs of the actually used JRE. The idea of multi-release JARs explicitly was to allow libraries (like Plexus Utils) to always use the latest APIs ASAP without breaking backwards compatibility. It would be disappointing to see Plexus Utils sticking with JRE 8 APIs in a JRE 21 world.

So the actual question is: Why not using the JDK 11 APIs in Plexus Utils?

(*) Disclaimer: I am part of the OpenJDK team working on I/O performance improvements, and I am the one who turned Plexus Utils into a multi-release JAR in 2021. My intention is to evangelize the use of modern Java's API so libraries will gain performance benefits automatically whenever we do an internal optimization inside of OpenJDK.

@michael-o
Copy link
Member

What superior performance?

Thank you for continuing the discussion about this PR. :-)

JDK 11's Files.readString allows the JRE to perform internal tricks like prevention of unnecessary data duplication. It does that by calling OpenJDK-specific APIs internally. Such optimizations are impossible for custom code, as these internal APIs are not public. Hence, JRE 8's API potentially creates comparably higher memory use and higher GC pressure, both resulting in worse performance. While this might not be an actual problem for every application (possibly it might not even be a problem for Plexus Utils), it OTOH neither harms any application to use JRE 11's API by default, whenever running on JRE 11+, hence this API should be used if any possible (at least this is the general idea why we(*) create new APIs in OpenJDK frequently). There is no good reason to not use the APIs of the actually used JRE. The idea of multi-release JARs explicitly was to allow libraries (like Plexus Utils) to always use the latest APIs ASAP without breaking backwards compatibility. It would be disappointing to see Plexus Utils sticking with JRE 8 APIs in a JRE 21 world.

So the actual question is: Why not using the JDK 11 APIs in Plexus Utils?

(*) Disclaimer: I am part of the OpenJDK team working on I/O performance improvements, and I am the one who turned Plexus Utils into a multi-release JAR in 2021. My intention is to evangelize the use of modern Java's API so libraries will gain performance benefits automatically whenever we do an internal optimization inside of OpenJDK.

Interesting and reasonable explanation.

@michael-o
Copy link
Member

@mkarg Please squash, I will test and merge.

@michael-o michael-o self-assigned this Jan 21, 2023
@mkarg
Copy link
Collaborator Author

mkarg commented Jan 21, 2023

@mkarg Please squash, I will test and merge.

Thank you. Squash is pushed.

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.

None yet

3 participants