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

Reply via email to