Thank you Alan for the further review!
On 11/27/19 12:53 PM, Alan Bateman wrote:
On 27/11/2019 11:52, Ivan Gerasimov wrote:
:
It's not clear how to distinguish the classes inside the java.base
module that do not have to depend on sun.nio.cs. If you feel strong
about NTLM and XML, I can replace sun.nio.cs.* instances with
corresponding java.charset.StandardCharsets.* constants in these
classes.
There isn't any way to say what is core and non-core in java.base so
you have to use your judgement. My personal view is that the NTLM,
XML, SOCKS and several others in this patch should stick to the
standard APIs if possible as their performance will likely be
dominated by other factors.
Personally, I think that using constants sun.nio.cs.xxx.INSTANCE is not
too bad even in the code that is unlikely to be executed during the VM
startup. One small advantage is that if the code is copy/pasted within
the java.base module, it will not bring the risk of early initialization
of StandardCharsets.
With NTLM, however, switching to StandardCharsets allows to remove
sun.nio.cs.UTF_16LE.INSTANCE and all other corresponding modifications.
So, I used StandardCharsets in NTLM (and in XML and SOCKS, as you
suggested), and left sun.nio.cs constants in all other places.
Here's the updated webrev:
http://cr.openjdk.java.net/~igerasim/8234147/02/webrev/
It builds fine, tests run fine.
--
With kind regards,
Ivan Gerasimov