Re: Review/comment needed for the new public java.util.Base64 class
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 more tuning. But my main question: Why don't you enqueue those coders in the well known sun.nio.cs.ext provider? Base64 encoding is not a charset. Mainly it is supposed to be byte[]-byte[] coding. Even for byte[]=char[]/String, Base64 encode byte[] to char[]/String and decode from char[]/String to byte[]. The wrong direction compared to charset en/decoder. At least, I think, class Base64 should be hosted in java.nio.charset rather than common java.util. As I believe, that the usage of this encoding is far from common you might be surprised how common it might be:-) -Sherman , adding it to the lazy loaded charsets.jar IMO should be more reasonable. -Ulf 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 (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: http://cr.openjdk.java.net/~sherman/4235519/score3 -Sherman
Re: Review/comment needed for the new public java.util.Base64 class
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 forgotten those tricks from sun.nio.cs coders? ;-) ) The implementation probably needs more tuning. Hopefully, some day we have arrays of primitive types in the class file's constant pool. But my main question: Why don't you enqueue those coders in the well known sun.nio.cs.ext provider? Base64 encoding is not a charset. Mainly it is supposed to be byte[]-byte[] coding. Even for byte[]=char[]/String, Base64 encode byte[] to char[]/String and decode from char[]/String to byte[]. The wrong direction compared to charset en/decoder. 1:0 for you. I only had the ASCII/Unicode characters - Base64 encoding in mind, which seems in fact a Java-String to byte[] to char[]/String 2-step-encoding. At least, I think, class Base64 should be hosted in java.nio.charset rather than common java.util. As I believe, that the usage of this encoding is far from common you might be surprised how common it might be:-) or surprised, why it wasn't part of the JDK before. -Ulf
Re: Review/comment needed for the new public java.util.Base64 class
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 flag in the parameters. If you have an endOfInput flag, you can use that in the condition instead of atom == 0. Consider this example where trying to encode 25 bytes using an input ByteBuffer of 16 bytes and an output ByteBuffer of 32 bytes (Of course, you'd use larger buffers in practice, but the same can happen with larger buffers too, as long as your buffer size is not divisible by three, or you do not completely fill the buffer with each read. Both can happen if the media you are reading from uses a different sector/block size - like when attaching a base64 attachment of a large file from disk to an email). First you fill 16 bytes of input into input buffer, then you call encode the first time. it will encode the first 15 bytes to 20 bytes of output, then encode the 16th byte to XY==. You flush the output and fill the next 9 bytes into the input buffer. The next call to encode will encode the 9 bytes to 12 bytes. So, in total you have 36 bytes, but the padding is in the middle. With the check, the first call will encode the first 15 bytes to 20 bytes of output and leave the 16th byte in the buffer. You flush the output and load 9 bytes to the 1 remaining byte in the input buffer. The second call will encode the first 9 bytes to 12 bytes and leave one byte left over. You flush the buffer and call a third time, which will encode the last byte to XY==. Still 36 bytes, this time with padding at the end. Regards, Michael
Re: Review/comment needed for the new public java.util.Base64 class
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, so it assumes all input will be passed in in the src buffer and only needs limit support for cases that the existing output buffer is not sufficient. We will probably have to go with the endOfInput flag approach, if the current version is still not enough for most of common use cases. -Sherman On 10/18/2012 11:01 AM, Michael Schierl wrote: 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 flag in the parameters. If you have an endOfInput flag, you can use that in the condition instead of atom == 0. Consider this example where trying to encode 25 bytes using an input ByteBuffer of 16 bytes and an output ByteBuffer of 32 bytes (Of course, you'd use larger buffers in practice, but the same can happen with larger buffers too, as long as your buffer size is not divisible by three, or you do not completely fill the buffer with each read. Both can happen if the media you are reading from uses a different sector/block size - like when attaching a base64 attachment of a large file from disk to an email). First you fill 16 bytes of input into input buffer, then you call encode the first time. it will encode the first 15 bytes to 20 bytes of output, then encode the 16th byte to XY==. You flush the output and fill the next 9 bytes into the input buffer. The next call to encode will encode the 9 bytes to 12 bytes. So, in total you have 36 bytes, but the padding is in the middle. With the check, the first call will encode the first 15 bytes to 20 bytes of output and leave the 16th byte in the buffer. You flush the output and load 9 bytes to the 1 remaining byte in the input buffer. The second call will encode the first 9 bytes to 12 bytes and leave one byte left over. You flush the buffer and call a third time, which will encode the last byte to XY==. Still 36 bytes, this time with padding at the end. Regards, Michael
Re: Review/comment needed for the new public java.util.Base64 class
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 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: http://cr.openjdk.java.net/~sherman/4235519/score3 -Sherman
bug fix for native kerberos libraries
Hello,
This simple fix allows kerberos authentication to work with:
-Dsun.security.jgss.native=true
and microsoft's sqljdbc 4.0.2206.100 driver.
Enjoy,
christos
--- a/java/src/sun/security/jgss/GSSUtil.java Mon Oct 15 17:43:08 2012 -0400
+++ b/java/src/sun/security/jgss/GSSUtil.java Mon Oct 15 17:44:28 2012 -0400
@@ -333,10 +333,19 @@
Subject accSubj = Subject.getSubject(acc);
VectorGSSCredentialSpi result = null;
if (accSubj != null) {
-result = new VectorGSSCredentialSpi();
IteratorGSSCredentialImpl iterator =
accSubj.getPrivateCredentials
(GSSCredentialImpl.class).iterator();
+// GSSCredentialImpl is only implemented in
+// the non-native kerberos implementation,
+// so if we don't get any elements here
+// assume native and return null so that
+// searchSubject does not fail. A better
+// fix is to implement the code that handles
+// this in native java.
+if (!iterator.hasNext())
+return null;
+result = new VectorGSSCredentialSpi();
while (iterator.hasNext()) {
GSSCredentialImpl cred = iterator.next();
debug(...Found cred + cred);
Re: bug fix for native kerberos libraries
(Forwarding to security-dev as this should be discussed in that group, not
core-libs).
On 10/18/12 5:02 PM, chris...@zoulas.com wrote:
Hello,
This simple fix allows kerberos authentication to work with:
-Dsun.security.jgss.native=true
and microsoft's sqljdbc 4.0.2206.100 driver.
Enjoy,
christos
--- a/java/src/sun/security/jgss/GSSUtil.java Mon Oct 15 17:43:08 2012 -0400
+++ b/java/src/sun/security/jgss/GSSUtil.java Mon Oct 15 17:44:28 2012 -0400
@@ -333,10 +333,19 @@
Subject accSubj = Subject.getSubject(acc);
VectorGSSCredentialSpi result = null;
if (accSubj != null) {
-result = new VectorGSSCredentialSpi();
IteratorGSSCredentialImpl iterator =
accSubj.getPrivateCredentials
(GSSCredentialImpl.class).iterator();
+// GSSCredentialImpl is only implemented in
+// the non-native kerberos implementation,
+// so if we don't get any elements here
+// assume native and return null so that
+// searchSubject does not fail. A better
+// fix is to implement the code that handles
+// this in native java.
+if (!iterator.hasNext())
+return null;
+result = new VectorGSSCredentialSpi();
while (iterator.hasNext()) {
GSSCredentialImpl cred = iterator.next();
debug(...Found cred + cred);
Re: Review/comment needed for the new public java.util.Base64 class
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. webrev: http://cr.openjdk.java.net/~sherman/4235519/webrev some performance scores: http://cr.openjdk.java.net/~sherman/4235519/score3 -Sherman Have you done any performance tuning on the new methods that encode/decode into an existing ByteBuffer? Just wondering as the direct buffer case seems more expensive that I would expect. It would be interesting to see using the put(int,byte) and fixing up the position at the end would help. It might also be something to look at with the compiler folks. get(index) and put(index, byte) do help, see 20%+ improvement. sherman@sherman-linux:~/Workspace/jdk8/test/java/util/Base64$ java PermBase64 20 1000 j.u.Base64.encode(ba) : 702000 j.u.Base64.encode(bb) : 690024 j.u.Base64.encode(bb, bb) : 910582 j.u.Base64.encode(bb, bb)-D: 1198606 migBase64.encode(ba) : 661271 -- j.u.Base64.decode(ba) : 797152 j.u.Base64.decode(bb) : 802968 j.u.Base64.decode(bb, bb) : 999577 j.u.Base64.decode(bb, bb)-D: 1472003 migBase64.decode(ba) : 1458450 webrev has been updated accordingly. Now implementations of de/encodeArray and de/encodeBuffer are identical except the access operation. -Sherman
RFR (XXS): 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
http://cr.openjdk.java.net/~twisti/6771058 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp Reviewed-by: This test can fail if finalizer of Basic is not called for some reason. It happens in compiled mode if compilation is more slow than execution of the main loop. The fix is to call System.runFinalization() after System.gc() to ensure that finalizer is run. test/java/lang/ref/Basic.java
Re: RFR (XXS): 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
On Oct 18, 2012, at 3:31 PM, Mandy Chung mandy.ch...@oracle.com wrote: Hi Christian, Thanks for taking on this bug. Sure. I'm trying to get HotSpot nightly failures down. Just curious - the test runs with a max of 10 GCs. You reproduced this bug on a slower machine with fastdebug build. If you increase the max number of GCs, I wonder how long the test will take to complete/pass? Around 14 GCs (runtime with -Xcomp approximately 40 seconds on that particular machine). With your fix, how many GC does it take to complete? I suspect it's one. Between 2 and 3 GCs. I am guessing that the test might want to test that the finalizers are being invoked during GC and references are dequeued. I wonder if there is another alternate fix instead of forcing the finalizers to be run. Just a thought. The fix was not my idea. I just picked the bug and tried the suggested fix and it worked. The other suggested fix is to increase the sleep time (but I don't like that). -- Chris Mandy On 10/18/2012 2:43 PM, Christian Thalinger wrote: http://cr.openjdk.java.net/~twisti/6771058 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp Reviewed-by: This test can fail if finalizer of Basic is not called for some reason. It happens in compiled mode if compilation is more slow than execution of the main loop. The fix is to call System.runFinalization() after System.gc() to ensure that finalizer is run. test/java/lang/ref/Basic.java
Re: RFR (XXS): 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
Hi Christian, Thanks for taking on this bug. Just curious - the test runs with a max of 10 GCs. You reproduced this bug on a slower machine with fastdebug build. If you increase the max number of GCs, I wonder how long the test will take to complete/pass? With your fix, how many GC does it take to complete? I suspect it's one. I am guessing that the test might want to test that the finalizers are being invoked during GC and references are dequeued. I wonder if there is another alternate fix instead of forcing the finalizers to be run. Just a thought. Mandy On 10/18/2012 2:43 PM, Christian Thalinger wrote: http://cr.openjdk.java.net/~twisti/6771058 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp Reviewed-by: This test can fail if finalizer of Basic is not called for some reason. It happens in compiled mode if compilation is more slow than execution of the main loop. The fix is to call System.runFinalization() after System.gc() to ensure that finalizer is run. test/java/lang/ref/Basic.java
Re: RFR (XXS): 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
On 10/18/2012 3:37 PM, Christian Thalinger wrote: Just curious - the test runs with a max of 10 GCs. You reproduced this bug on a slower machine with fastdebug build. If you increase the max number of GCs, I wonder how long the test will take to complete/pass? Around 14 GCs (runtime with -Xcomp approximately 40 seconds on that particular machine). Would increasing the max to 20 fix this bug? I don't have a strong objection to call System.runFinalization. As I don't know what behavior the test wants to cover, adjusting the number of max GC invocations seems a good alternative. Mandy
Re: RFR (XXS): 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
Hi Chris,
On 19/10/2012 8:37 AM, Christian Thalinger wrote:
On Oct 18, 2012, at 3:31 PM, Mandy Chungmandy.ch...@oracle.com wrote:
Just curious - the test runs with a max of 10 GCs. You
reproduced this bug on a slower machine with fastdebug build.
If you increase the max number of GCs, I wonder how long
the test will take to complete/pass?
Around 14 GCs (runtime with -Xcomp approximately 40 seconds on that particular
machine).
With your fix, how many GC does it take to complete?
I suspect it's one.
Between 2 and 3 GCs.
I believe it takes two GC cycles to move a finalizable object into the
finalization queue.
I am guessing that the test might
want to test that the finalizers are being invoked during
GC and references are dequeued. I wonder if there is
another alternate fix instead of forcing the finalizers
to be run. Just a thought.
The fix was not my idea. I just picked the bug and tried the suggested fix and
it worked. The other suggested fix is to increase the sleep time (but I don't
like that).
The key here is what the test is trying to exercise. From this comment:
/* Cause a dummy object to be finalized, since the finalizer thread
might retain a reference to the Basic instance after it's been
finalized (this happens with java_g) */
we can see there is some intended interaction with the (primary)
finalizer thread. By calling runFinalization() we're introducing the
secondary finalizer thread and performing synchronous finalization -
which may (or may not) negate the intended interaction with the primary
finalizer thread.
There have been recent changes to other tests that require finalization
to basically add explicit loops testing a variable that is set by the
finalizer eg:
while (!obj.finalized) {
System.gc();
Thread.sleep(100);
}
It may be that this could be added to the ClearFinalizerThread object?
Unfortunately it can sometimes be very easy to fix these tests in a
way that negates the purpose of the test.
David
-- Chris
Mandy
On 10/18/2012 2:43 PM, Christian Thalinger wrote:
http://cr.openjdk.java.net/~twisti/6771058
6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
Reviewed-by:
This test can fail if finalizer of Basic is not called for some
reason. It happens in compiled mode if compilation is more slow than
execution of the main loop.
The fix is to call System.runFinalization() after System.gc() to
ensure that finalizer is run.
test/java/lang/ref/Basic.java
Re: RFR (XXS): 6771058: TEST_BUG: java/lang/ref/Basic.java may fail with -server -Xcomp
Hi Martin, On 19/10/2012 10:58 AM, Martin Buchholz wrote: http://code.google.com/p/guava-libraries/source/browse/guava-testlib/src/com/google/common/testing/GcFinalization.java That code uses runFinalization in places which means it is not exercising the primary finalization mechanism. But looking further into the whole thing I don't think it really makes much difference semantically afterall. All that changes is which thread executes the finalizer - the object comes from the same ReferenceQueue regardless. Which means that runFinalization() will really only help if the main finalizer thread is delayed executing another finalizer - which for some tests may be caused by compilation of the finalize method. So Chris/Mandy: I think the original suggested fix is fine here. Cheers, David
Re: [7u12] Request for approval: 7198073: (prefs) user prefs not saved [macosx]
Approved for jdk7u-dev. regards, Sean. On 18/10/2012 13:34, Kurchi Hazra wrote: This is a request for approval to backport the fix for 7198073 from 8 to 7u12. Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7198073 Webrev: http://cr.openjdk.java.net/~khazra/7198073/7u12/webrev/ This change been reviewed by Alan Bateman [1] and the fix has been pushed into jdk8 [2]. The fix for 7u12 is identical. Thanks, Kurchi [1]http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-October/011767.html [2]http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3a6b76a468bd
