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
