On 27/11/2019 04:39, Ivan Gerasimov wrote:
Hello!
It is a cleanup fix with mostly two kinds of changes:
- when a standard charset is specified by its name, use a
preinitialized Charset constant instead,
- replace the usage of StandardCharset.* constants with their
sun.nio.cs.* equivalents to avoid accidental early initialization of
rarely-used charsets.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8234147
WEBREV: http://cr.openjdk.java.net/~igerasim/8234147/00/webrev/
I also had to modify one regression test that relied on a private
auxiliary method, which was removed with the fix.
Mach5 control build looks green.
Would you please help review the fix?
The motivation is okay and most of the changes are okay but it does
create a needless dependency on sun.nio.* classes in places that are
unlikely (or impossible) to use during early startup. I think the change
to the JDBC CachedRow implementation, prefs, and the jdk.net.httpserver
module should be dropped as want fewer classes outside of java.base
depending on java.base internals, not more. I also wonder about the
NTLM, XML properties and a few more that shouldn't be dependency on
sun.nio.* classes if possible.
Minot nit but if this is pushed then it would be good to keep the
imports consistent with the existing style where you can, e.g. I see
several files where the import sun.nio.* is put at the top of source
files that already group imports of JDK internal classes together
further down.
Mandy might want to comment on TrySetAccessibleTest. If I'm not mistaken
it should say "non-exported" rather than "exported". Also I think we
want this test to have a dependency on a few JDK internal and private
methods as possible as it's a maintenance issue to keep it up to date
(as you've experienced here with the removal of Perf.getBytes).
-Alan
