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

Remove some methods from recognized methods #20520

Closed
wants to merge 1 commit into from

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Nov 6, 2024

These methods no longer exist in the Java class library.

  • sun/nio/cs/ISO_8859_1$Decoder.decodeISO8859_1()
  • sun/nio/cs/UTF_8$Encoder.encodeUTF_8()
  • sun/nio/cs/ext/SBCS_Encoder.encodeSBCS()
  • sun/nio/cs/ext/SBCS_Decoder.decodeSBCS()

@knn-k knn-k added the comp:jit label Nov 6, 2024
@knn-k
Copy link
Contributor Author

knn-k commented Nov 6, 2024

Jenkins test sanity amac jdk17 depends eclipse-omr/omr#7527

@hzongaro
Copy link
Member

Note that these enumerated constants are used in EncodeMethods which is in turn used to identify recognized methods in:

I don't think these methods exist in any of those classes.

I think this change looks good, but I'm just trying to look into the history to understand when these methods disappeared.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 12, 2024

@knn-k
Copy link
Contributor Author

knn-k commented Nov 12, 2024

Removing sun/nio/cs/UTF_16$Encoder.encodeUTF16Big() and encodeUTF_16Little() will be a big change, removing helper functions for p and x. I would like to do that in separate pull requests after making sure it is safe to remove them.

I wonder if someone working on Z can look at sun/nio/cs/ext/SBCS_*.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 13, 2024

IBM SDK Java 8 has the class sun/nio/cs/ext/SBCS_Encoder, but it does not have the encodeSBCS() method. It is the same for the decodeSBCS() method in the SBCS_Decoder class.

@knn-k knn-k force-pushed the removeEncodeDecodeMethods branch 2 times, most recently from aa25505 to dd01061 Compare November 14, 2024 05:09
@knn-k
Copy link
Contributor Author

knn-k commented Nov 14, 2024

I updated the commit to additionally remove SBCS_Encoder.encodeSBCS() and SBCS_Decoder.decodeSBCS().

@knn-k
Copy link
Contributor Author

knn-k commented Nov 14, 2024

Jenkins test sanity zlinux jdk17 depends eclipse-omr/omr#7527

@knn-k
Copy link
Contributor Author

knn-k commented Nov 18, 2024

I opened separate pull requests for sun/nio/cs/UTF16_Encoder methods:

@knn-k
Copy link
Contributor Author

knn-k commented Nov 29, 2024

Rebased to resolve merge conflicts.

These methods no longer exist in the Java class library.

- sun/nio/cs/ISO_8859_1$Decoder.decodeISO8859_1()
- sun/nio/cs/UTF_8$Encoder.encodeUTF_8()
- sun/nio/cs/ext/SBCS_Encoder.encodeSBCS()
- sun/nio/cs/ext/SBCS_Decoder.decodeSBCS()

Signed-off-by: KONNO Kazuhiro <[email protected]>
@knn-k knn-k force-pushed the removeEncodeDecodeMethods branch from be97564 to e8d610b Compare December 16, 2024 05:01
@knn-k
Copy link
Contributor Author

knn-k commented Dec 16, 2024

Jenkins test sanity xlinux,zlinux jdk17 depends eclipse-omr/omr#7527

Rebased to resolve merge conflicts.

@r30shah
Copy link
Contributor

r30shah commented Dec 19, 2024

Hi @knn-k , I am looking at the class files that is shipped with the IBM SDK 8 on z/OS to go through the methods that you are removing in this PR.

Commenting about following 3 recognized methods referencing from changes in runtime/compiler/codegen/J9RecognizedMethodsEnum.hpp which all exists in the IBM SDK8 on z/OS.

  1. sun.nio.cs.ISO_8859_1$Decoder.decodeISO8859_1(byte[], int, int, char[], int);
  2. sun.nio.cs.ext.SBCS_Encoder.encodeSBCS(char[], int, int, byte[], int, byte[]);
  3. sun.nio.cs.ext.SBCS_Decoder.decodeSBCS(byte[], int, int, char[], int, char[]);
  4. sun.nio.cs.UTF_8.$Encoder.encodeUTF_8(char[], int, int, byte[], int);

These changes in the JIT recognize methods provides vital performance related to code pages conversion and is important on z/OS that we should not remove.

@knn-k
Copy link
Contributor Author

knn-k commented Dec 20, 2024

@r30shah Thank you for checking IBM SDK 8 for z/OS. I am cancelling this PR.

I checked IBM SDK 8 for other platforms as well as Semeru Runtimes 8-21. I didn't know IBM SDK 8 for z/OS had its own classes and methods.

@knn-k knn-k closed this Dec 20, 2024
@knn-k knn-k deleted the removeEncodeDecodeMethods branch December 20, 2024 01:04
@pshipton
Copy link
Member

Maybe it would be worth adding comments for these methods to explain where they are used.

@knn-k
Copy link
Contributor Author

knn-k commented Dec 20, 2024

I opened PR #20864 for adding comments to the encoding/decoding methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants