Review/comment needed for the new public java.util.Base64 class

Xueming Shen xueming.shen at oracle.com
Tue Oct 30 22:40:42 UTC 2012


Hi Ulf, thanks for the comments.

Webrev has been updated to address most of your comments.

http://cr.openjdk.java.net/~sherman/4235519/webrev

(1) all @param and @return tags have been "normalized".
(2) private constructor BAse64() {} has been moved up.
(3) MIMELINEMAX and CRLF have been moved into encoder.
(4) -> lineLength >>2<<2;

Your questions:


>
> I'm "confused" about the order of xxcode() and Xxcoder.
> In other places e.g. charsets, we have de... before en..., which is 
> also true alphabetical
>
should not be an issue. javadoc output should be in alphabetic order. If 
it is really
concerns you, I can do a copy/paste:-)


>
>
> - What (should) happen(s), if lineSeparator.length == 0 ?
do nothing. expected?



> - Isn't one of these little faster, or at least more clear? :
>     lineLength -= lineLength % 4
>     lineLength & 0xFFFFFFFC
>     lineLength << 2 >> 2
>
>
>  603         static {
>  604             Arrays.fill(fromBase64, -1);
>  605             for (int i = 0; i < Encoder.toBase64.length; i++)
>  606                 fromBase64[Encoder.toBase64[i]] = i;
>  607             fromBase64['='] = -2;
>  608         }
> This causes the inner Encoder class to be loaded, even if not needed. 
> So maybe move the toBase64 table to the outer class.
> Have you compared performance with fromBase64 as byte[] (including 
> local 'b' in decode() as byte)?

understood. but it appears the hotspot likes it the constant/fixed 
length lookup
table is inside encoder. Same as you suggestion in your previous email 
to use
String in source and expand it during runtime. It saves about 500 bytes 
but slows
the server vm. Those repeating lines of "isURL? ...." is also supposed 
to be a
performance tweak to eliminate the array boundary check in loop.

>
>
> Why you mix the algorithms to check the padding? :
>  824                         if (b == -2) {   // padding byte
>  890                     if (b == '=') {
>
It is supposed reduce one "if" check for normal base64 character inside the
loop. I'm not that sure it really helps though.

-Sherman




More information about the core-libs-dev mailing list