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

Replace undocumented APIs with documented APIs in the OpenJDK #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dhanalla
Copy link

@dhanalla dhanalla commented Aug 8, 2024

The wepoll code has been ported from opensource repo wepoll to OpenJDK in PR pull/3816, which incorporated undocumented Windows APIs (NtCreateKeyedEvent, NtReleaseKeyedEvent, NtWaitForKeyedEvent).

This PR aims to replace these undocumented APIs with documented synchronization APIs.

Test Performed:

  1. All 12 tests in wepoll repo passed
    test-auto-drop-on-close PASS
    test-connect-fail-events PASS
    test-connect-success-events PASS
    test-ctl-fuzz PASS
    test-error PASS
    test-leak-1 PASS
    test-mixed-socket-types PASS
    test-multi-poll PASS
    test-oneshot-and-hangup PASS
    test-reflock PASS
    test-tree PASS
    test-udp-pings PASS
    DONE - 12 PASSED, 0 FAILED

  2. No new failure in JTreg test

  3. Micro bench results: similar performance score before and after the changes

without change in this PR:

image

With the change in this PR:

image

@swesonga
Copy link
Member

Should we also create an issue in the wepoll repo so that this issue can also be addressed at the source?

@dhanalla
Copy link
Author

Should we also create an issue in the wepoll repo so that this issue can also be addressed at the source?

Thanks @swesonga, The repository appears to be inactive, and no updates have been made since 11/30/2020.
The Visual Studio team opened PR #37 on wepoll branch, but there has been no response to it for several months.

Copy link

@JohnTortugo JohnTortugo left a comment

Choose a reason for hiding this comment

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

LGTM

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 this pull request may close these issues.

3 participants