Re: Using unsigned library work in the JDK

Thu, 26 Jan 2012 17:35:04 -0800

Sherman, thanks for offering to investigate the performance impact, if any, of this change. 

-Joe

PS All the jar/zip regression tests pass with this change.

On 01/26/2012 09:52 AM, Xueming Shen wrote:

Joe,

The changes look fine. However I have the same performance concern in
cases that the vm fails to inline the toUnsignedInt(), especially for those 
"simple" one line utility methods, such as the get16(), CH(), SH(), my
guess is that it might have a performance impact. It might not be a big
deal for operation like creating or extracting a jar/zip file but probably will 
slow down the loc/cen table reading only operation such as list a jar/zip
file. I will try to get some performance data to see if the impact is
"significant" enough to be a real concern.

-Sherman

On 01/25/2012 08:37 PM, Joe Darcy wrote:

Hello,
As a follow-up to the recent push of unsigned library support in the JDK [1], I grepped -i for "0xff" in the JDK code base to look for candidate locations to use the new methods. I choose to first tackle some jar/zip code. 

Sherman, can you review the changes below?
diff -r 303b67074666 src/share/classes/java/util/jar/JarOutputStream.java --- a/src/share/classes/java/util/jar/JarOutputStream.java Tue Jan 24 15:13:27 2012 -0500 +++ b/src/share/classes/java/util/jar/JarOutputStream.java Wed Jan 25 20:31:05 2012 -0800 
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -135,7 +135,7 @@
      * The bytes are assumed to be in Intel (little-endian) byte order.
      */
     private static int get16(byte[] b, int off) {
-        return (b[off] & 0xff) | ((b[off+1] & 0xff) << 8);
+ return Byte.toUnsignedInt(b[off]) | ( Byte.toUnsignedInt(b[off+1]) << 8); 
     }

     /*
diff -r 303b67074666 src/share/classes/java/util/jar/Manifest.java
--- a/src/share/classes/java/util/jar/Manifest.java Tue Jan 24 15:13:27 2012 -0500 +++ b/src/share/classes/java/util/jar/Manifest.java Wed Jan 25 20:31:05 2012 -0800 
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -339,7 +339,7 @@
                     return -1;
                 }
             }
-            return buf[pos++] & 0xff;
+            return Byte.toUnsignedInt(buf[pos++]);
         }
public int read(byte[] b, int off, int len) throws IOException { diff -r 303b67074666 src/share/classes/java/util/zip/InflaterInputStream.java --- a/src/share/classes/java/util/zip/InflaterInputStream.java Tue Jan 24 15:13:27 2012 -0500 +++ b/src/share/classes/java/util/zip/InflaterInputStream.java Wed Jan 25 20:31:05 2012 -0800 
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights reserved. 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -119,7 +119,7 @@
      */
     public int read() throws IOException {
         ensureOpen();
- return read(singleByteBuf, 0, 1) == -1 ? -1 : singleByteBuf[0] & 0xff; + return read(singleByteBuf, 0, 1) == -1 ? -1 : Byte.toUnsignedInt(singleByteBuf[0]); 
     }

     /**
diff -r 303b67074666 src/share/classes/java/util/zip/ZipInputStream.java
--- a/src/share/classes/java/util/zip/ZipInputStream.java Tue Jan 24 15:13:27 2012 -0500 +++ b/src/share/classes/java/util/zip/ZipInputStream.java Wed Jan 25 20:31:05 2012 -0800 
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2009, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights reserved. 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -435,7 +435,7 @@
      * The bytes are assumed to be in Intel (little-endian) byte order.
      */
     private static final int get16(byte b[], int off) {
-        return (b[off] & 0xff) | ((b[off+1] & 0xff) << 8);
+ return Byte.toUnsignedInt(b[off]) | (Byte.toUnsignedInt(b[off+1]) << 8); 
     }

     /*
diff -r 303b67074666 src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipConstants.java --- a/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipConstants.java Tue Jan 24 15:13:27 2012 -0500 +++ b/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipConstants.java Wed Jan 25 20:31:05 2012 -0800 
@@ -185,11 +185,11 @@
      */
     ///////////////////////////////////////////////////////
     static final int CH(byte[] b, int n) {
-       return b[n] & 0xff;
+        return Byte.toUnsignedInt(b[n]);
     }

     static final int SH(byte[] b, int n) {
-        return (b[n] & 0xff) | ((b[n + 1] & 0xff) << 8);
+ return Byte.toUnsignedInt(b[n]) | (Byte.toUnsignedInt(b[n + 1]) << 8); 
     }

     static final long LG(byte[] b, int n) {

If the changes look good, I'll file a bug for them, etc.
In case other people want to look over candidates sites in different areas, I've included the list of JDK files using "0xff" below. 

Thanks,

-Joe
[1] 4504839: Java libraries should provide support for unsigned integer arithmetic http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-January/008926.html 

.

Reply via email to