Using unsigned library work in the JDK (original) (raw)

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