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.
Thanks, done. Unfortunately, imports conventions are not very
consistent across the JDK code, so I mostly tried to preserve the
pre-existing order of imports in each file.
Okay.
If I got it correct, the test was meant to access a private static
method, and after removing Perf.getBytes there were no such methods
left in the Perf class. That's why I had to pick some other method to
test, so I chose ModulePath.packageName instead.
The test is trying to cover all the possible accessibilities. The
"exported package" descriptions are a bit confusing but are explained by
the test running--add-exports. My only concern with the change is that
it expands the dependences on internals to classes in 3 packages, it
means anyone touching those classes may have to fix up this test.
So overall I think this patch is okay, just me trying to avoid coupling
when we can.
-Alan
