-
Notifications
You must be signed in to change notification settings - Fork 74
jdk11 AIX PollsetSelector implementation #1652
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
base: master
Are you sure you want to change the base?
jdk11 AIX PollsetSelector implementation #1652
Conversation
|
Jenkins doc stage |
|
@pshipton - I have updated the topics. Please check and let me know if the changes are ok. Thanks! |
docs/configuring.md
Outdated
|
|
||
| ##  Configuring the NIO selector provider (AIX only) | ||
|
|
||
| In the 0.57.0 release, a new New Input/Output (NIO) channels selector provider, which is based on pollset system facility, is implemented on AIX. NIO selectors are used for scalable I/O operations, allowing a single thread to monitor multiple channels. The pollset facility is an AIX-specific system call that efficiently handles multiple file descriptors. The new NIO selector provider configuration provides significant performance benefits when reading from, writing to, or managing a steady number of network sockets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
channels -> channel
based on pollset -> based on the pollset
Same changes apply to version0.57.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest removing "steady" from the final sentence here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keithc-ca - Would it better to say "...managing multiple network sockets"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works for me.
docs/configuring.md
Outdated
|
|
||
| In the 0.57.0 release, a new New Input/Output (NIO) channels selector provider, which is based on pollset system facility, is implemented on AIX. NIO selectors are used for scalable I/O operations, allowing a single thread to monitor multiple channels. The pollset facility is an AIX-specific system call that efficiently handles multiple file descriptors. The new NIO selector provider configuration provides significant performance benefits when reading from, writing to, or managing a steady number of network sockets. | ||
|
|
||
| This feature is available only optionally at run time and not by default. You can configure it by specifying the new parameter `sun.nio.ch.PollsetSelectorProvider` in the `-Djava.nio.channels.spi.SelectorProvider` system property on the command line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"only" can be removed.
21a40a0 to
3972237
Compare
docs/version0.57.md
Outdated
|
|
||
| ###  A new NIO selector provider implementation is available for AIX | ||
|
|
||
| A new New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX. The new implementation provides significant performance benefits when reading from, writing to, or managing a steady number of network sockets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest removing "steady" here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it better to say "...managing multiple network sockets"?
3972237 to
1275de5
Compare
|
Jenkins doc stage |
|
@pshipton @keithc-ca - I have changed the sentence to "...managing multiple network sockets". Hope that is ok. Also, I have assumed that the Btw, I don't see a separate topic for the |
zl-wang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
docs/configuring.md
Outdated
|
|
||
| In the 0.57.0 release, a new New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX. NIO selectors are used for scalable I/O operations, allowing a single thread to monitor multiple channels. The pollset facility is an AIX-specific system call that efficiently handles multiple file descriptors. The new NIO selector provider configuration provides significant performance benefits when reading from, writing to, or managing multiple network sockets. | ||
|
|
||
| This feature is available optionally at run time and not by default. You can configure it by specifying the new parameter `sun.nio.ch.PollsetSelectorProvider` in the `-Djava.nio.channels.spi.SelectorProvider` system property on the command line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest "value" would be better than "parameter" here.
docs/version0.57.md
Outdated
|
|
||
| A new New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX. The new implementation provides significant performance benefits when reading from, writing to, or managing multiple network sockets. | ||
|
|
||
| This feature is available optionally at run time and not by default. You can configure it by specifying the new parameter `sun.nio.ch.PollsetSelectorProvider` in the `-Djava.nio.channels.spi.SelectorProvider` system property on the command line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, parameter -> value.
It's documented in OpenJDK API documentation: |
docs/configuring.md
Outdated
|
|
||
| ##  Configuring the NIO selector provider (AIX only) | ||
|
|
||
| In the 0.57.0 release, a new New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX. NIO selectors are used for scalable I/O operations, allowing a single thread to monitor multiple channels. The pollset facility is an AIX-specific system call that efficiently handles multiple file descriptors. The new NIO selector provider configuration provides significant performance benefits when reading from, writing to, or managing multiple network sockets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This contains "new New", I think "new" can be removed. Also it would read better as:
a New Input/Output (NIO) channel selector provider is implemented on AIX, which is based on the pollset system facility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even for "new NIO" the "new" can be removed. It's new today, but it won't be in 3 months.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "new New" sequence made me pause. Perhaps it would read better as:
an NIO (New Input/Output) channel selector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This contains "new New", I think "new" can be removed. Also it would read better as:
a New Input/Output (NIO) channel selector provider is implemented on AIX, which is based on the pollset system facility.
@pshipton @keithc-ca - Yes, this "new New" did make me pause too, but I was not sure whether there were any other NIO channel provider or not. I will change this.
@pshipton - But, the "a New Input/Output (NIO) channel selector provider is implemented on AIX, which is based on the pollset system facility" might be misinterpreted as that the AIX itself is based on the pollset system facility, rather than the NIO provider. The " which is based on the pollset..." should be near "...provider" because that sentence modifies the "provider" not "AIX". (I am assuming that it is the NIO channel security provider that is based on the pollset facility.) I would strongly recommend to keep the sentence as
"...a New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "new New" sequence made me pause. Perhaps it would read better as:
an NIO (New Input/Output) channel selector
As a rule, we usually provide the expanded form first and then place the abbreviation in the brackets. I have changed the sentence to read
...a New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX.
eclipse-openj9#1648 New parameter related to NIO selector provider added for `-Djava.nio.channels.spi.SelectorProvider` system property. Review comment incorporated. Closes eclipse-openj9#1648 Signed-off-by: Sreekala Gopakumar [email protected]
1275de5 to
83c1904
Compare
|
Jenkins doc stage |
|
|
||
| A New Input/Output (NIO) channel selector provider, which is based on the pollset system facility, is implemented on AIX. The new implementation provides significant performance benefits when reading from, writing to, or managing multiple network sockets. | ||
|
|
||
| This feature is available optionally at run time and not by default. You can configure it by specifying the new value `sun.nio.ch.PollsetSelectorProvider` in the `-Djava.nio.channels.spi.SelectorProvider` system property on the command line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a nit, but the system property name does not start with -D.
The phrase "optionally at run time" suggests that the choice can be made after the VM starts which is not the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, what is the correct system property name? Also, in our existing documents, all the system properties have "-D". Is this system property different?
Regarding the 2nd change, how about:
This feature is available at run time only and not by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The system property name is "java.nio.channels.spi.SelectorProvider". I think we should use the same style as in https://github.com/eclipse-openj9/openj9-docs/blob/master/docs/version0.56.md which says
the format of the
java.vm.versionsystem property value
or https://github.com/eclipse-openj9/openj9-docs/blob/master/docs/vgclog.md which says
the system property
file.encodingis set tobestfit936
It might look like this: This feature is not enabled by default; to enable it, set the value of the system property java.nio.channels.spi.SelectorProvider to sun.nio.ch.PollsetSelectorProvider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keithc-ca - Should we also specify that this is possible at run time only and not after the VM starts?
This feature is not enabled by default. To enable it, set the value of the system property
java.nio.channels.spi.SelectorProvidertosun.nio.ch.PollsetSelectorProviderat run time.
Also, should -Djava.nio.channels.spi.SelectorProvider=sun.nio.ch.PollsetSelectorProvider be changed or is it correct?
#1648
New parameter related to NIO selector provider added for
-Djava.nio.channels.spi.SelectorProvidersystem property.Closes #1648
Signed-off-by: Sreekala Gopakumar [email protected]