RFR 8234147 : Avoid looking up standard charsets in core libraries
Alan Bateman
Alan.Bateman at oracle.com
Wed Nov 27 07:54:33 UTC 2019
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
More information about the core-libs-dev
mailing list