Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8256245: AArch64: Implement Base64 decoding intrinsic #3228

Closed
wants to merge 12 commits into from
@@ -5798,16 +5798,17 @@ class StubGenerator: public StubCodeGenerator {
__ br(Assembler::LT, Process4B);

// The 1st character of the input can be illegal if the data is MIME encoded.

This comment has been minimized.

Loading
@theRealAph

theRealAph Apr 6, 2021
Contributor Outdated

Why is this sentence here? It is very misleading.

This comment has been minimized.

Loading
@dgbo

dgbo Apr 7, 2021
Author Member Outdated

This sentence was used to describe the worst case observed frequently so that readers can understand more easily why the pre-processing non-SIMD code is necessary.
I apologize for being unclear and misleading. The annotations have been modified as suggested.

// We cannot benefits from SIMD for this case. The max line size of MIME
// We cannot benefit from SIMD for this case. The max line size of MIME
// encoding is 76, with the PreProcess80B blob, we actually use no-simd
This conversation was marked as resolved by dgbo

This comment has been minimized.

Loading
@theRealAph

theRealAph Apr 2, 2021
Contributor Outdated

"cannot benefit"

This comment has been minimized.

Loading
@theRealAph

theRealAph Apr 6, 2021
Contributor Outdated

OK, so I now understand what is actually going on here, and it has nothing to do with illegal first characters.
The problem is that the maximum block length the decode will be supplied with is 76 bytes, and there isn't enough time for the SIMD to be worthwhile. So the comment should be "In the MIME case, the line length cannot be more than 76 bytes (see RFC 2045.) This is too short a block for SIMD to be worthwhile, so we use non-SIMD here."

// instructions for all MIME encoded data.
__ movw(rscratch1, 79);

__ BIND(Process4B);
__ ldrb(r10, __ post(src, 1));
__ ldrb(r11, __ post(src, 1));
__ ldrb(r12, __ post(src, 1));
__ ldrb(r13, __ post(src, 1));
__ ldrw(r14, __ post(src, 4));
__ ubfxw(r10, r14, 0, 8);
__ ubfxw(r11, r14, 8, 8);
__ ubfxw(r12, r14, 16, 8);
__ ubfxw(r13, r14, 24, 8);
// get the de-code

This comment has been minimized.

Loading
@theRealAph

theRealAph Apr 2, 2021
Contributor

Four loads and four post increments rather than one load and a few BFMs? Why?

__ ldrb(r10, Address(nosimd_codec, r10, Address::uxtw(0)));
__ ldrb(r11, Address(nosimd_codec, r11, Address::uxtw(0)));
@@ -38,8 +38,8 @@

private Base64.Encoder encoder, mimeEncoder;
private Base64.Decoder decoder, mimeDecoder;
private ArrayList<byte[]> encoded, mimeEncoded;
private byte[] decoded, mimeDecoded;
private ArrayList<byte[]> encoded, mimeEncoded, errorEncoded;
private byte[] decoded, mimeDecoded, errorDecoded;

private static final int TESTSIZE = 1000;

@@ -52,6 +52,11 @@

private byte[] lineSeparator = {'\r', '\n'};

/* Other value can be tested by passing parameters to the JMH
tests: -p errorIndex=3,64,144,208,272,1000,20000. */
@Param({"144"})
private int errorIndex;

@Setup
public void setup() {
Random r = new Random(1123);
@@ -66,6 +71,9 @@ public void setup() {
mimeDecoder = Base64.getMimeDecoder();
mimeEncoded = new ArrayList<byte[]> ();

errorDecoded = new byte[errorIndex + 100];
errorEncoded = new ArrayList<byte[]> ();

for (int i = 0; i < TESTSIZE; i++) {
int srcLen = 1 + r.nextInt(maxNumBytes);
byte[] src = new byte[srcLen];
@@ -80,6 +88,14 @@ public void setup() {
r.nextBytes(mimeSrc);
mimeEncoder.encode(mimeSrc, mimeDst);
mimeEncoded.add(mimeDst);

int errorSrcLen = errorIndex + r.nextInt(100);
byte[] errorSrc = new byte[errorSrcLen];
byte[] errorDst = new byte[(errorSrcLen + 2) / 3 * 4];
r.nextBytes(errorSrc);
encoder.encode(errorSrc, errorDst);
errorEncoded.add(errorDst);
errorDst[errorIndex] = (byte) '?';
}
}

This comment has been minimized.

Loading
@theRealAph

theRealAph Apr 2, 2021
Contributor

Are there any existing test cases for failing inputs?

@@ -100,4 +116,17 @@ public void testBase64MIMEDecode(Blackhole bh) {
bh.consume(mimeDecoded);
}
}

@Benchmark
@OperationsPerInvocation(TESTSIZE)
public void testBase64WithErrorInputsDecode (Blackhole bh) {
for (byte[] s : errorEncoded) {
try {
decoder.decode(s, errorDecoded);
bh.consume(errorDecoded);
} catch (IllegalArgumentException e) {
bh.consume(e);
}
}
}
}