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

Stop recognizing UTF16_Encoder.encodeUTF16 methods #20613

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

knn-k
Copy link
Contributor

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

This commit removes the code for encodeUTF16Big() and encodeUTF16Little() methods in sun.nio.cs.UTF16_Encoder from method recognition, and removes related helper functions.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 18, 2024

Jenkins test sanity plinux,,xlinux,zlinux jdk11

@knn-k knn-k changed the title Stop recognizing encodeUTF16 methods Stop recognizing UTF16.encodeUTF16 methods Nov 18, 2024
@knn-k knn-k changed the title Stop recognizing UTF16.encodeUTF16 methods Stop recognizing UTF_16.encodeUTF16 methods Nov 18, 2024
@knn-k knn-k force-pushed the removeEncodeUTF16 branch from 9e92901 to 1a4ccdc Compare November 18, 2024 06:49
@knn-k
Copy link
Contributor Author

knn-k commented Nov 18, 2024

Jenkins test sanity plinux,,xlinux,zlinux jdk11

@knn-k
Copy link
Contributor Author

knn-k commented Nov 18, 2024

Jenkins test sanity plinux,,xlinux,zlinux jdk11 depends eclipse-omr/omr#7548

@knn-k knn-k marked this pull request as ready for review November 18, 2024 11:57
@knn-k knn-k requested a review from dsouzai as a code owner November 18, 2024 11:57
@knn-k
Copy link
Contributor Author

knn-k commented Nov 18, 2024

@hzongaro FYI.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 19, 2024

sun.nio.cs.UTF_16$Encoder no longer has the methods encodeUTF16Big() and encodeUTF16Little().
See Issue #20520.

@hzongaro hzongaro self-assigned this Nov 26, 2024
Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

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

It think the changes look good. May I ask you to correct the name of the class in the commit message? I believe it was named sun.nio.cs.UTF16_Encoder

runtime/compiler/env/j9method.cpp Show resolved Hide resolved
This commit removes the code for encodeUTF16Big() and encodeUTF16Little()
methods in sun.nio.cs.UTF16_Encoder from method recognition, and removes
related helper functions.

Signed-off-by: KONNO Kazuhiro <[email protected]>
@knn-k knn-k force-pushed the removeEncodeUTF16 branch from 1a4ccdc to 5e8ada3 Compare November 27, 2024 05:10
@knn-k knn-k changed the title Stop recognizing UTF_16.encodeUTF16 methods Stop recognizing UTF16_Encoder.encodeUTF16 methods Nov 27, 2024
@knn-k
Copy link
Contributor Author

knn-k commented Nov 27, 2024

I corrected the class name in the commit title and the commit message.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 27, 2024

Jenkins test sanity plinux,xlinux,zlinux jdk11 depends eclipse-omr/omr#7548

@knn-k
Copy link
Contributor Author

knn-k commented Nov 27, 2024

The test failure on x390x_linux looks the same as Issue #13756.

Copy link
Member

@hzongaro hzongaro 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. Thanks!

@hzongaro
Copy link
Member

As this will require a coordinated merge with eclipse-omr/omr#7548, I will run further testing as required by the Guide for Committers.

Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk11,jdk17,jdk21 depends eclipse-omr/omr#7548

@knn-k
Copy link
Contributor Author

knn-k commented Nov 28, 2024

Shall I split the PRs into smaller ones so that they don't require a coordinated merge?

@hzongaro
Copy link
Member

Shall I split the PRs into smaller ones so that they don't require a coordinated merge?

No worries! Let's just go ahead with these PRs as they are.

Windows builds are out of commission at the moment, so those failures are expected. I will restart the other failing builds.

Jenkins test sanity.functional xmac jdk8 depends eclipse-omr/omr#7548

@hzongaro
Copy link
Member

Jenkins test sanity.openjdk xmac jdk17,jdk21 depends eclipse-omr/omr#7548

@hzongaro
Copy link
Member

Jenkins test sanity.functional plinux jdk11,jdk21 depends eclipse-omr/omr#7548

@hzongaro
Copy link
Member

Failures:

@hzongaro
Copy link
Member

The only failures appear to be due to known issues. I will perform the coordinated merge with eclipse-omr/omr#7548 as soon as the same commit appears at the tips of the openj9 and master branches of the eclipse-openj9/openj9-omr repository and the master branch of eclipse-omr/omr repository.

@hzongaro hzongaro merged commit b5ee59c into eclipse-openj9:master Nov 29, 2024
79 of 89 checks passed
@knn-k knn-k deleted the removeEncodeUTF16 branch November 29, 2024 21:57
knn-k added a commit to knn-k/openj9 that referenced this pull request Dec 2, 2024
This commit restores the changes for encodeUTF16Big and encodeUTF16Little
that were removed by eclipse-openj9#20613.

Signed-off-by: KONNO Kazuhiro <[email protected]>
@knn-k
Copy link
Contributor Author

knn-k commented Dec 2, 2024

My apologies.
I realized that the sun.nio.cs.UTF16_Encoder methods are still used in IBM SDK Java 8. I thought the class name was UTF_16 from the enum symbol sun_nio_cs_UTF_16_Encoder_encodeUTF16*, and did not check UTF16_Encoder. I should have noticed earlier when @hzongaro added this comment: #20613 (review)
I am preparing PRs for reverting the changes by this PR and eclipse-omr/omr#7548.

@knn-k
Copy link
Contributor Author

knn-k commented Dec 2, 2024

I opened PRs #20722, eclipse-omr/omr#7577, and eclipse-omr/omr#7578 for reverting the changes of UTF16_Encoder.encodeUTF16 methods.

@hzongaro
Copy link
Member

hzongaro commented Dec 3, 2024

I realized that the sun.nio.cs.UTF16_Encoder methods are still used in IBM SDK Java 8

@knn-k, thank you for catching this! I'm sorry that I missed that while reviewing.

matthewhall2 pushed a commit to matthewhall2/openj9 that referenced this pull request Dec 16, 2024
This commit restores the changes for encodeUTF16Big and encodeUTF16Little
that were removed by eclipse-openj9#20613.

Signed-off-by: KONNO Kazuhiro <[email protected]>
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.

2 participants