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
.
