On Tue, 16 Feb 2021 15:05:33 GMT, Alan Bateman <al...@openjdk.org> wrote:

>>> > Is there a reason 
>>> > `sun.nio.cs.ISO_8859_1.Encoder#implEncodeISOArray(char[], int, byte[], 
>>> > int, int)` wasn't moved to `JavaLangAccess` as well?
>>> 
>>> Exposing StringUTF16.compress for Latin-1 and ASCII-compatible encoders 
>>> seem very reasonable, which I was thinking of exploring next as a separate 
>>> RFE.
>> 
>> Maybe I misunderstood. The intrinsified method you point out here pre-dates 
>> the work in JDK 9 to similarly intrinsify char[]->byte[] compaction in 
>> StringUTF16, see https://bugs.openjdk.java.net/browse/JDK-6896617
>> 
>> It might be worthwhile cleaning this up. Not having to route via 
>> SharedSecrets -> JavaLangAccess does speed things up during 
>> startup/interpretation, at the cost of some code duplication.
>
> This looks a really good change and it's great can these decoders get the 
> benefit of the instrinics work.
> 
> ISO_8869_1.decodeArrayLoop using JLA.inflate looks a bit strange and maybe we 
> can rename the method in the shared secret to inflatedCopy or just copy. or 
> just add a comment. The reason is that code outside of java.lang shouldn't 
> know anything about String's internals and inflation.
> 
> The reformatting changes to StreamEncoder/StreamDecoder make it hard to see 
> if there are any actual changes. Is there anything we should look at? It's 
> okay to include this cleanup, I can't tell what happened with this source 
> file.
> 
> I want to study the change System.setJavaLangAccess a bit further to 
> understand why this is needed (I know there is a awkward bootstrapping issue 
> here).
> 
> I expect Naoto will want to review this PR too.

I added the `Inflated copy from byte[] to char[], as defined by StringLatin1` 
comment to the `inflate` method in `JavaLangAccess`, but you're probably right 
and it would be good to make the name more explicit when exporting it outside 
of the package internal use. How about `inflateBytesToChars`?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2574

Reply via email to