RFR(M): 8248188: [PATCH] Add HotSpotIntrinsicCandidate and API for Base64 decoding

[Corey Ashford]

On 9/4/20 8:07 AM, Roger Riggs wrote:
> Hi Corey,
> 
> The idea I had in mind is refactoring the fast path into the method you 
> call decodeBlock.
> Base64: lines 751-768.
> 
> It leaves all the unknown/illegal character handling to the Java code.
> And yes, it does not need to handle MIME, except to return on illegal 
> characters.
> 
> The patch is attached.

Ah, I see what you mean now, and thanks for the patch!  The patch as 
presented doesn't work, however, because the intrinsic processes fewer 
bytes than are in the src buffer, and then executes a "continue;", which 
then proceeds to loop infinitely because the intrinsic won't process any 
more bytes after that.

I tried dropping the continue, but that doesn't work because the Java 
(non-intrinsic) code processes all of the bytes, and the line of code 
following the loop accesses one byte after the end of the src buffer 
causing an array bounds error.

So this needs to be re-thought a little, but it shouldn't be too 
difficult.  I will work on it.

Regards,

- Corey

> 
> Regards, Roger
> 
> 
> 
> On 8/31/20 6:22 PM, Corey Ashford wrote:
>> On 8/29/20 1:19 PM, Corey Ashford wrote:
>>> Hi Roger,
>>>
>>> Thanks for your reply and thoughts!  Comments interspersed below:
>>>
>>> On 8/28/20 10:54 AM, Roger Riggs wrote:
>> ...
>>>> Comparing with the way that the Base64 encoder was intrinsified, the
>>>> method that is intrinsified should have a method body that does
>>>> the same function, so it is interchangable.  That likely will just 
>>>> shift
>>>> the "fast path" code into the decodeBlock method.
>>>> Keeping the symmetry between encoder and decoder will
>>>> make it easier to maintain the code.
>>>
>>> Good point.  I'll investigate what this looks like in terms of the 
>>> actual code, and will report back (perhaps in a new webrev).
>>>
>>
>> Having looked at this again, I don't think it makes sense.  One thing 
>> that differs significantly from the encodeBlock intrinsic is that the 
>> decodeBlock intrinsic only needs to process a prefix of the data, and 
>> so it can leave virtually any amount of data at the end of the src 
>> buffer unprocessed, where as with the encodeBlock intrinsic, if it 
>> exists, it must process the entire buffer.
>>
>> In the (common) case where the decodeBlock intrinsic returns not 
>> having processed everything, it still needs to call the Java code, and 
>> if that Java code is "replaced" by the intrinsic, it's inaccessible.
>>
>> Is there something I'm overlooking here?  Basically I want the decode 
>> API to behave differently than the encode API, mostly to make the 
>> arch-specific intrinsic easier to implement. If that's not acceptable, 
>> then I need to rethink the API, and also figure out how to deal with 
>> the illegal character case.  The latter could perhaps be done by 
>> throwing an exception from the intrinsic, or maybe by returning a 
>> negative length that specifies the index of the illegal src byte, and 
>> then have the Java code throw the exception).
>>
>> Regards,
>>
>> - Corey
>>
>