Using unsigned library work in the JDK

Joe Darcy joe.darcy at oracle.com
Tue Feb 7 05:33:55 UTC 2012


Thanks Sherman; given the at most marginal performance cost, I'll push 
the changes in the next day or two.

-Joe


On 02/06/2012 05:02 PM, Xueming Shen wrote:
> Joe,
>
> I did some non-scientific measure of the performance. test1() is to 
> measure the change
> in a real "listing" operation on a ZipInputStream and test2() is a 
> pure access on the byte[]
> directly. I don't see any significant difference on performance in 
> both  -client and -server
> vm. I would assume the change has no performance impact to the real 
> zip access.
>
> In -client vm
> test1:  108 vs 109
> test2:  280 : 283
>
> Simple test case attached (the input is a big enough jar file, such as 
> the rt.jar)
>
> -Sherman
>
> --------------------------------------------
>
>     public static void main(String[] args) throws IOException {
>
>         byte[] data = java.nio.file.Files.readAllBytes(new 
> File(args[0]).toPath());
>         ByteArrayInputStream bais = new ByteArrayInputStream(data);
>
>         //warm up
>         test1(bais, 100);
>         System.out.printf("t=%d%n", test1(bais, 500));
>
>         test2(data, 100);
>         System.out.printf("t=%d%n", test2(data, 100));
>     }
>
>     static long test1(ByteArrayInputStream bais, int n) throws 
> IOException {
>         long t = 0;
>         for (int i = 0; i < n; i++) {
>             bais.reset();
>
>             ZipInputStream zis = new ZipInputStream(bais);
>             ZipEntry e;
>             long t0 = System.currentTimeMillis();
>             while ((e = zis.getNextEntry()) != null) {
>                 zis.closeEntry();
>                 //System.out.println(new String(e.getName()));
>             }
>             long t1 = System.currentTimeMillis();
>             t += t1 - t0;
>         }
>         return t /= n;
>     }
>
>
>     static long test2(byte data[], int n) throws IOException {
>         int j = 10000;
>         long t = 0;
>         if (j >= data.length -1)
>             j = data.length -2;
>         for (int m = 0; m < n; m++) {
>             long t0 = System.nanoTime();
>             int sum = 0;
>             for (int i = 0; i < j; i++) {
>                 sum += get16(data, i);
>             }
>             long t1 = System.nanoTime();
>             t += t1 - t0;
>         }
>         return t / n / 100;
>     }
>
>     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);
>
>     }
>
>
> On 1/26/2012 5:33 PM, Joe Darcy wrote:
>> 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 
>>>>
>>>>
>>>> .
>>




More information about the core-libs-dev mailing list