On Tue, 22 Sep 2020 02:45:36 GMT, CoreyAshford 
<github.com+51754783+coreyashf...@openjdk.org> wrote:

> This patch set encompasses the following commits:
> 
> - Adds a new HotSpot intrinsic candidate to the java.lang.Base64 class - 
> decodeBlock(), and provides a flexible API for
>   the intrinsic.  The API is similar to the existing encodeBlock intrinsic.
> - Adds the code in HotSpot to check and martial the new intrinsic's arguments 
> to the arch-specific intrinsic
>   implementation
> - Adds a Power64LE-specific implementation of the decodeBlock intrinsic.
> - Adds a JMH microbenchmark for both Base64 encoding and encoding.
> - Enhances the JTReg hotspot intrinsic "TestBase64.java" regression test to 
> more fully test both decoding and encoding.

Changes requested by rriggs (Reviewer).

src/java.base/share/classes/java/util/Base64.java line 747:

> 745:          * Decodes base64 characters, and returns the number of data 
> bytes
> 746:          * produced by the base64 decode intrinsic.
> 747:          *

"intrinsic" can be omitted. Both the java and the intrinsic produce the same 
output.

src/java.base/share/classes/java/util/Base64.java line 759:

> 757:          * src, it must process a multiple of four of them, making the
> 758:          * returned destination length a multiple of three.
> 759:          *

If the dst array is too short does the intrinsic stop short or throw?
The java code would throw an ArrayIndexOutOfBoundsException.
It should not occur since the java code allocates the proper buffer but...
It might be worth mentioning.

src/java.base/share/classes/java/util/Base64.java line 820:

> 818:                     dp += dl;
> 819:                 }
> 820:                 if (sp == sl) {

I'd rather see  (sp >= s1) instead of the equality, its would be consistent 
with the condition of the while loop.
(And is already lines 767-768).

test/hotspot/jtreg/compiler/intrinsics/base64/TestBase64.java line 260:

> 258:     }
> 259:
> 260:     private static final byte[] nonBase64 = {

Please add a comment describing this test data.
(It looks like it could be generated more compactly than an explicit array).
Ditto nonBase64URL below.

test/hotspot/jtreg/compiler/intrinsics/base64/TestBase64.java line 240:

> 238:     }
> 239:
> 240:     private static byte[] hexStringToByteArray(String s) {

A method to convert a Hex String to a byte array already exists in 
test/lib/jdk/test/lib/Utils.java  in  toByteArray(s).
Add "jdk.test.lib.Utils" to the "@build" line at the top of the test.

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

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

Reply via email to