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

2012-11-30 Thread Ulf Zibis
Hi Sherman, is this ssue still open ? -Ulf Am 03.11.2012 21:33, schrieb Ulf Zibis: Am 30.10.2012 23:40, schrieb Xueming Shen: 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

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

2012-11-30 Thread Xueming Shen
Ulf, The base64 implementation is in TL right now. It does address some of the issue you suggested in this email. And I remember I did try use byte[] alphabets, which I don't recall bring us any benefit, but I'm not sure in which setup. The latest is at

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

2012-11-03 Thread Ulf Zibis
Am 30.10.2012 23:40, schrieb Xueming Shen: 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

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

2012-10-30 Thread Ulf Zibis
Am 24.10.2012 04:56, schrieb Xueming Shen: Thanks for the review. I hope I addressed most of the comments in the updated webrev at http://cr.openjdk.java.net/~sherman/4235519/webrev Hi Sherman, my additional comments: I'm confused about the order of xxcode() and Xxcoder. In other places

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

2012-10-30 Thread Ulf Zibis
Oops: lineLength 2 2 I meant:: lineLength 2 2 -Ulf

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

2012-10-30 Thread Ulf Zibis
Hi Sherman, thanks for your details. Has this discussion been on the list, and I have missed it? I see a problem with hiding the singleton choice: Developers might tend to repetitively invoke public static Encoder getEncoder(int lineLength, byte[] lineSeparator) instead reusing the the same

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

2012-10-30 Thread Xueming Shen
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

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

2012-10-29 Thread Ulf Zibis
Hi Sherman, can you give me a short answer please? -Ulf Am 23.10.2012 16:57, schrieb Ulf Zibis: Am 23.10.2012 15:04, schrieb Alan Bateman: I'm not sure that getUrlEncoder is the most suitable name to get a base64url encoder. The reason is that the method name makes it sound like it returns a

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

2012-10-29 Thread Xueming Shen
Ulf, my apology. Some how I missed your email. We tried various prototypes for this simple utility class. See http://cr.openjdk.java.net/~sherman/base64/ The v4 might be close to the static constant approach you suggested. While It's hard to draw a clear line on which one is better, we

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

2012-10-23 Thread Alan Bateman
On 18/10/2012 03:10, Xueming Shen wrote: : webrev: http://cr.openjdk.java.net/~sherman/4235519/webrev I took another pass over this, focusing on the API as that is what we have to get right. Performance is important too but I think the priority has to be the API first. Overall I think it is

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

2012-10-23 Thread Ulf Zibis
Am 23.10.2012 15:04, schrieb Alan Bateman: I'm not sure that getUrlEncoder is the most suitable name to get a base64url encoder. The reason is that the method name makes it sound like it returns a URLEncoder or or at least an encoder for HTML forms. While more verbose, getBase64UrlEncoder is

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

2012-10-23 Thread Xueming Shen
Hi Alan, Thanks for the review. I hope I addressed most of the comments in the updated webrev at http://cr.openjdk.java.net/~sherman/4235519/webrev mainly (1) Pulled the base64 terms up to the class doc and then be referenced from various methods (2) Gave up the C style de/encode(byte[],

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

2012-10-23 Thread Xueming Shen
On 10/23/2012 8:50 PM, Mike Duigou wrote: I'm eager to use this! Some comments: -ttcode/tt is used in some places rather than {@code } fixed - The initialul would perhaps be better as adl. I don't know if definition lists are allowed in javadoc. - public OutputStream wrap(OutputStream

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

2012-10-18 Thread Xueming Shen
On 10/18/2012 6:19 AM, Ulf Zibis wrote: Hi Sherman, you could create the code table by help of a String constant, which would speed up the loading and save a little footprint on the class file. (you have forgotten those tricks from sun.nio.cs coders? ;-) ) The implementation probably needs

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

2012-10-18 Thread Ulf Zibis
Am 18.10.2012 16:04, schrieb Xueming Shen: On 10/18/2012 6:19 AM, Ulf Zibis wrote: Oops, you are working at 6 in the morning? Hi Sherman, you could create the code table by help of a String constant, which would speed up the loading and save a little footprint on the class file. (you have

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

2012-10-18 Thread Michael Schierl
Am 18.10.2012 04:10, schrieb Xueming Shen: Hi Webrev has been updated with following changes (1) added a pair of en/decode(ByteBuffer src, ByteBuffer dst) methods I think line 443 needs an additional check for atom == 0. Same for the array case. At least if you do not want an endOfInput

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

2012-10-18 Thread Xueming Shen
Michael, The current approach assumes that the base64 en/decoding of a file, stream will be handled by the en/decoder.wrap(...). The en/decode(bb, bb) is basically for one time invocation input and output from/to existing buffers, with the hope of avoiding the coding-life-circle management,

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

2012-10-18 Thread Mike Duigou
I wonder if there would be advantage in using a SharedSecrets mechanism to allow construction of a String directly from a char array. The intermediate byte array seems wasteful especially for what is likely to be a heavily used path. Mike On Oct 17 2012, at 19:10 , Xueming Shen wrote: Hi

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

2012-10-18 Thread Xueming Shen
On 10/18/2012 07:43 AM, Alan Bateman wrote: On 18/10/2012 03:10, Xueming Shen wrote: Hi Webrev has been updated with following changes (1) added a pair of en/decode(ByteBuffer src, ByteBuffer dst) methods (2) some minor spec clarification regarding the end of decoding (3) performance tuning.

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

2012-10-17 Thread Xueming Shen
Hi Webrev has been updated with following changes (1) added a pair of en/decode(ByteBuffer src, ByteBuffer dst) methods (2) some minor spec clarification regarding the end of decoding (3) performance tuning. webrev: http://cr.openjdk.java.net/~sherman/4235519/webrev some performance scores:

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

2012-10-13 Thread Peter Levart
One way to do it without state in encoder is 1st as suggested to have the 'endOfStream' flag and in addition when new-line breaks are requested, to only encode full lines at each invocation. So If one wants to use this line-breakage feature he/she must make sure that there is enough space

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

2012-10-13 Thread Xueming Shen
With moving some code piece around (moved those lookup table into En/Decoder) and some array access code tuning (with the hope of eliminating the array boundary access) in encode0(), it appears the hotspot now can better optimize the decoder loop for the -server mode for encoding. decode0

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

2012-10-13 Thread Xueming Shen
On 10/13/12 10:21 AM, Xueming Shen wrote: With moving some code piece around (moved those lookup table into En/Decoder) and some array access code tuning (with the hope of eliminating the array boundary access) in encode0(), it appears the hotspot now can better optimize the decoder loop for

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

2012-10-12 Thread Xueming Shen
On 10/11/2012 02:43 AM, Stephen Colebourne wrote: There are lots of other public base 64 implementations to test/check against. http://commons.apache.org/net/api-3.1/org/apache/commons/net/util/Base64.html http://www.cs.berkeley.edu/~jonah/bc/org/bouncycastle/util/encoders/Base64.html

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

2012-10-12 Thread Alan Bateman
On 11/10/2012 19:30, Ariel Weisberg wrote: : I know that ByteBuffers are pain, but I did notice that you can't specify a source/dest pair when using ByteBuffers and that ByteBuffers without arrays have to be copied. I don't see a simple safe way to normalize access to them the way you can if

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

2012-10-12 Thread Michael Schierl
Hello, (sorry if the threading is broken, but I was not subscribed to the list and only found the discussion on Twitter and read it in the mailing list archive) Ariel Weisberg wrote on Thu Oct 11 11:30:56 PDT 2012: I know that ByteBuffers are pain, but I did notice that you can't specify a

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

2012-10-11 Thread Alan Bateman
On 11/10/2012 04:15, Xueming Shen wrote: There is no plan yet. The sun.misc.BASE64En/Decoder should already trigger a compiler warning for it's a sun private API. @Deprecated annotation might be a good fit. -Sherman Yes, I think we should deprecate them. The other thing suggested in the JEP

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

2012-10-11 Thread Stephen Colebourne
The class level javadoc is quite short. It also has hyperlinks in the first sentence, which means that they are visible in package level javadoc. Consider having no hyperlinks in the first sentence, and expanding a little on what base 64 is. There are lots of other public base 64 implementations

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

2012-10-11 Thread Xueming Shen
On 10/11/2012 02:43 AM, Stephen Colebourne wrote: The class level javadoc is quite short. It also has hyperlinks in the first sentence, which means that they are visible in package level javadoc. Consider having no hyperlinks in the first sentence, and expanding a little on what base 64 is.

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

2012-10-10 Thread Xueming Shen
A standard/public API for Base64 encoding and decoding has been long overdue. JDK8 has a JEP [1] for this particular request. Here is the draft proposal to add a public Base64 utility class for JDK8. http://cr.openjdk.java.net/~sherman/4235519/webrev This class basically provides 4 variants

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

2012-10-10 Thread Iris Clark
Message- From: Xueming Shen Sent: Wednesday, October 10, 2012 10:55 AM To: core-libs-dev Subject: Review/comment needed for the new public java.util.Base64 class A standard/public API for Base64 encoding and decoding has been long overdue. JDK8 has a JEP [1] for this particular request. Here

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

2012-10-10 Thread Joe Darcy
for the new public java.util.Base64 class A standard/public API for Base64 encoding and decoding has been long overdue. JDK8 has a JEP [1] for this particular request. Here is the draft proposal to add a public Base64 utility class for JDK8. http://cr.openjdk.java.net/~sherman/4235519/webrev

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

2012-10-10 Thread Weijun Wang
Several questions: 1. In encode0(byte[] src, byte[] dst) 281 if (linepos == linemax (atom != 0 || sp sl)) { Maybe atom != 0 is not necessary? 2. Is it necessary to explicitly mention in the spec that there is no CrLf at the end of a MIME encoded string? 3. The test

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

2012-10-10 Thread Xueming Shen
On 10/10/12 6:51 PM, Weijun Wang wrote: Several questions: 1. In encode0(byte[] src, byte[] dst) 281 if (linepos == linemax (atom != 0 || sp sl)) { Maybe atom != 0 is not necessary? The logic here is that if we reached the last atom (atom == 0), but if there is still

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

2012-10-10 Thread Xueming Shen
a fine opportunity to use the @Deprecated annotation... -Joe Thanks, iris -Original Message- From: Xueming Shen Sent: Wednesday, October 10, 2012 10:55 AM To: core-libs-dev Subject: Review/comment needed for the new public java.util.Base64 class A standard/public API for Base64 encoding

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

2012-10-10 Thread Weijun Wang
On 10/11/2012 11:09 AM, Xueming Shen wrote: On 10/10/12 6:51 PM, Weijun Wang wrote: Several questions: 1. In encode0(byte[] src, byte[] dst) 281 if (linepos == linemax (atom != 0 || sp sl)) { Maybe atom != 0 is not necessary? The logic here is that if we reached the

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

2012-10-10 Thread Xueming Shen
On 10/10/12 8:16 PM, Weijun Wang wrote: On 10/11/2012 11:09 AM, Xueming Shen wrote: On 10/10/12 6:51 PM, Weijun Wang wrote: Several questions: 1. In encode0(byte[] src, byte[] dst) 281 if (linepos == linemax (atom != 0 || sp sl)) { Maybe atom != 0 is not necessary?

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

2012-10-10 Thread Weijun Wang
On 10/11/2012 11:32 AM, Xueming Shen wrote: On 10/10/12 8:16 PM, Weijun Wang wrote: On 10/11/2012 11:09 AM, Xueming Shen wrote: On 10/10/12 6:51 PM, Weijun Wang wrote: Several questions: 1. In encode0(byte[] src, byte[] dst) 281 if (linepos == linemax (atom != 0 || sp

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

2012-10-10 Thread Xueming Shen
On 10/10/12 8:39 PM, Weijun Wang wrote: On 10/11/2012 11:32 AM, Xueming Shen wrote: On 10/10/12 8:16 PM, Weijun Wang wrote: On 10/11/2012 11:09 AM, Xueming Shen wrote: On 10/10/12 6:51 PM, Weijun Wang wrote: Several questions: 1. In encode0(byte[] src, byte[] dst) 281