Re: ObjectIn/OutputStream improvements
Forwarding to correct core-libs-dev list. -Chris. On 31 Jan 2014, at 14:22, Chris Hegarty <chris.hegarty@oracle.com> wrote:
Hi Robert,
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
http://cr.openjdk.java.net/~chegar/serial_stupp.00/webrev/
Specifically, * I think for clarify and readability SerialCallbackContext checkAndSetUsed() should be invoked directly. It makes it very clear what the intent is. * Skip primitive/object reading/writing if no primitives/objects in the stream/class. ( I personally don't see any benefit of rounding up the size of the array, it just seems to add unnecessary complexity ) * Move primitive types into getPrimitiveSignature for better reuse of code. This retains your change to not create the additional StringBuilder when it is not necessary.
I am currently running tests on this change.
-Chris.
On 07/01/14 13:03, Robert Stupp wrote:
Hi, I've attached the diff to the original email - it has been stripped. Here's a second try (inline). I've already signed the OCA and it has been approved :) Robert # HG changeset patch # User snazy # Date 1387101091 -3600 # Sun Dec 15 10:51:31 2013 +0100 # Node ID 6d46d99212453017c88417678d08dc8f10da9606 # Parent 9e1be800420265e5dcf264a7ed4abb6f230dd19d Removed some unnecessary variable assignments. ObjectInputStream: - skip primitive/object reading if no primitive/objects in class - use shared StringBuilder for string reading (prevent superfluous object allocations) ObjectOutputStream: - skip primitive/object writing if no primitive/objects in class - use unsafe access to calculate UTF-length - use unsafe access in readBytes() and writeChars() methods to access String value field - removed cbuf field ObjectStreamClass/ObjectStreamField: - minor improvement in getClassSignature ; share method code with ObjectStreamField diff --git a/src/share/classes/java/io/ObjectInputStream.java b/src/share/classes/java/io/ObjectInputStream.java --- a/src/share/classes/java/io/ObjectInputStream.java +++ b/src/share/classes/java/io/ObjectInputStream.java @@ -39,8 +39,8 @@ import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; import static java.io.ObjectStreamClass.processQueue; + import sun.reflect.misc.ReflectUtil;
/** @@ -534,7 +534,7 @@ if (ctx == null) { throw new NotActiveException("not in call to readObject"); } - Object curObj = ctx.getObj(); + ctx.getObj(); ObjectStreamClass curDesc = ctx.getDesc(); bin.setBlockDataMode(false); GetFieldImpl getField = new GetFieldImpl(curDesc); @@ -1597,7 +1597,7 @@ int descHandle = handles.assign(unshared ? unsharedMarker : desc); passHandle = NULL_HANDLE;
- ObjectStreamClass readDesc = null; + ObjectStreamClass readDesc; try { readDesc = readClassDescriptor(); } catch (ClassNotFoundException ex) { @@ -1976,29 +1976,34 @@ }
int primDataSize = desc.getPrimDataSize(); - if (primVals == null || primVals.length < primDataSize) { - primVals = new byte[primDataSize]; - } - bin.readFully(primVals, 0, primDataSize, false); - if (obj != null) { - desc.setPrimFieldValues(obj, primVals); - } - - int objHandle = passHandle; - ObjectStreamField[] fields = desc.getFields(false); - Object[] objVals = new Object[desc.getNumObjFields()]; - int numPrimFields = fields.length - objVals.length; - for (int i = 0; i < objVals.length; i++) { - ObjectStreamField f = fields[numPrimFields + i]; - objVals[i] = readObject0(f.isUnshared()); - if (f.getField() != null) { - handles.markDependency(objHandle, passHandle); + if (primDataSize > 0) { + if (primVals == null || primVals.length < primDataSize) { + primVals = new byte[ ((primDataSize>>4)+1)<<4 ]; + } + bin.readFully(primVals, 0, primDataSize, false); + if (obj != null) { + desc.setPrimFieldValues(obj, primVals); } } - if (obj != null) { - desc.setObjFieldValues(obj, objVals); + + int numObjFields = desc.getNumObjFields(); + if (numObjFields > 0) { + int objHandle = passHandle; + ObjectStreamField[] fields = desc.getFields(false); + Object[] objVals = new Object[numObjFields]; + int numPrimFields = fields.length - objVals.length; + for (int i = 0; i < objVals.length; i++) { + ObjectStreamField f = fields[numPrimFields + i]; + objVals[i] = readObject0(f.isUnshared()); + if (f.getField() != null) { + handles.markDependency(objHandle, passHandle); + } + } + if (obj != null) { + desc.setObjFieldValues(obj, objVals); + } + passHandle = objHandle; } - passHandle = objHandle; }
/** @@ -2377,8 +2382,10 @@ private final byte[] buf = new byte[MAX_BLOCK_SIZE]; /** buffer for reading block data headers */ private final byte[] hbuf = new byte[MAX_HEADER_SIZE]; - /** char buffer for fast string reads */ + /** char buffer for fast string reads - used by {@link #readUTFSpan(long)} */ private final char[] cbuf = new char[CHAR_BUF_SIZE]; + /** shared string builder for less object allocations - used by {@link #readUTFBody(long)}, {@link #readUTFChar(long)} and {@link #readUTFSpan(long)} */ + private final StringBuilder sbuf = new StringBuilder(CHAR_BUF_SIZE);
/** block data mode */ private boolean blkmode = false; @@ -3044,19 +3051,19 @@ * utflen bytes. */ private String readUTFBody(long utflen) throws IOException { - StringBuilder sbuf = new StringBuilder(); if (!blkmode) { end = pos = 0; }
+ sbuf.setLength(0); while (utflen > 0) { int avail = end - pos; if (avail >= 3 || (long) avail == utflen) { - utflen -= readUTFSpan(sbuf, utflen); + utflen -= readUTFSpan(utflen); } else { if (blkmode) { // near block boundary, read one byte at a time - utflen -= readUTFChar(sbuf, utflen); + utflen -= readUTFChar(utflen); } else { // shift and refill buffer manually if (avail > 0) { @@ -3076,10 +3083,10 @@ * Reads span of UTF-encoded characters out of internal buffer * (starting at offset pos and ending at or before offset end), * consuming no more than utflen bytes. Appends read characters to - * sbuf. Returns the number of bytes consumed. + * {@link #sbuf}. Returns the number of bytes consumed. */ - private long readUTFSpan(StringBuilder sbuf, long utflen) - throws IOException + private long readUTFSpan(long utflen) + throws IOException { int cpos = 0; int start = pos; @@ -3111,19 +3118,19 @@ throw new UTFDataFormatException(); } cbuf[cpos++] = (char) (((b1 & 0x1F) << 6) | - ((b2 & 0x3F) << 0)); + (b2 & 0x3F)); break;
case 14: // 3 byte format: 1110xxxx 10xxxxxx 10xxxxxx b3 = buf[pos + 1]; - b2 = buf[pos + 0]; + b2 = buf[pos ]; pos += 2; if ((b2 & 0xC0) != 0x80 || (b3 & 0xC0) != 0x80) { throw new UTFDataFormatException(); } cbuf[cpos++] = (char) (((b1 & 0x0F) << 12) | - ((b2 & 0x3F) << 6) | - ((b3 & 0x3F) << 0)); + ((b2 & 0x3F) << 6) | + (b3 & 0x3F)); break;
default: // 10xx xxxx, 1111 xxxx @@ -3150,12 +3157,12 @@
/** * Reads in single UTF-encoded character one byte at a time, appends - * the character to sbuf, and returns the number of bytes consumed. + * the character to {@link #sbuf}, and returns the number of bytes consumed. * This method is used when reading in UTF strings written in block * data mode to handle UTF-encoded characters which (potentially) * straddle block-data boundaries. */ - private int readUTFChar(StringBuilder sbuf, long utflen) + private int readUTFChar(long utflen) throws IOException { int b1, b2, b3; @@ -3181,8 +3188,7 @@ if ((b2 & 0xC0) != 0x80) { throw new UTFDataFormatException(); } - sbuf.append((char) (((b1 & 0x1F) << 6) | - ((b2 & 0x3F) << 0))); + sbuf.append((char) (((b1 & 0x1F) << 6) | (b2 & 0x3F))); return 2;
case 14: // 3 byte format: 1110xxxx 10xxxxxx 10xxxxxx @@ -3198,8 +3204,8 @@ throw new UTFDataFormatException(); } sbuf.append((char) (((b1 & 0x0F) << 12) | - ((b2 & 0x3F) << 6) | - ((b3 & 0x3F) << 0))); + ((b2 & 0x3F) << 6) | + (b3 & 0x3F))); return 3;
default: // 10xx xxxx, 1111 xxxx diff --git a/src/share/classes/java/io/ObjectOutputStream.java b/src/share/classes/java/io/ObjectOutputStream.java --- a/src/share/classes/java/io/ObjectOutputStream.java +++ b/src/share/classes/java/io/ObjectOutputStream.java @@ -35,7 +35,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import static java.io.ObjectStreamClass.processQueue; -import java.io.SerialCallbackContext; + +import sun.misc.Unsafe; import sun.reflect.misc.ReflectUtil;
/** @@ -458,7 +459,7 @@ if (ctx == null) { throw new NotActiveException("not in call to writeObject"); } - Object curObj = ctx.getObj(); + ctx.getObj(); ObjectStreamClass curDesc = ctx.getDesc(); curPut = new PutFieldImpl(curDesc); } @@ -1300,7 +1301,7 @@ */ private void writeString(String str, boolean unshared) throws IOException { handles.assign(unshared ? null : str); - long utflen = bout.getUTFLength(str); + long utflen = BlockDataOutputStream.getUTFLength(str); if (utflen <= 0xFFFF) { bout.writeByte(TC_STRING); bout.writeUTF(str, utflen); @@ -1527,29 +1528,34 @@ desc.checkDefaultSerialize();
int primDataSize = desc.getPrimDataSize(); - if (primVals == null || primVals.length < primDataSize) { - primVals = new byte[primDataSize]; + if (primDataSize > 0) { + if (primVals == null || primVals.length < primDataSize) { + primVals = new byte[ ((primDataSize>>4)+1)<<4 ]; + } + desc.getPrimFieldValues(obj, primVals); + bout.write(primVals, 0, primDataSize, false); } - desc.getPrimFieldValues(obj, primVals); - bout.write(primVals, 0, primDataSize, false);
- ObjectStreamField[] fields = desc.getFields(false); - Object[] objVals = new Object[desc.getNumObjFields()]; - int numPrimFields = fields.length - objVals.length; - desc.getObjFieldValues(obj, objVals); - for (int i = 0; i < objVals.length; i++) { - if (extendedDebugInfo) { - debugInfoStack.push( - "field (class \"" + desc.getName() + "\", name: \"" + - fields[numPrimFields + i].getName() + "\", type: \"" + - fields[numPrimFields + i].getType() + "\")"); - } - try { - writeObject0(objVals[i], - fields[numPrimFields + i].isUnshared()); - } finally { + int numObjFields = desc.getNumObjFields(); + if (numObjFields > 0) { + ObjectStreamField[] fields = desc.getFields(false); + Object[] objVals = new Object[numObjFields]; + int numPrimFields = fields.length - objVals.length; + desc.getObjFieldValues(obj, objVals); + for (int i = 0; i < objVals.length; i++) { if (extendedDebugInfo) { - debugInfoStack.pop(); + debugInfoStack.push( + "field (class \"" + desc.getName() + "\", name: \"" + + fields[numPrimFields + i].getName() + "\", type: \"" + + fields[numPrimFields + i].getType() + "\")"); + } + try { + writeObject0(objVals[i], + fields[numPrimFields + i].isUnshared()); + } finally { + if (extendedDebugInfo) { + debugInfoStack.pop(); + } } } } @@ -1743,15 +1749,11 @@ private static final int MAX_BLOCK_SIZE = 1024; /** maximum data block header length */ private static final int MAX_HEADER_SIZE = 5; - /** (tunable) length of char buffer (for writing strings) */ - private static final int CHAR_BUF_SIZE = 256;
/** buffer for writing general/block data */ private final byte[] buf = new byte[MAX_BLOCK_SIZE]; /** buffer for writing block data headers */ private final byte[] hbuf = new byte[MAX_HEADER_SIZE]; - /** char buffer for fast string writes */ - private final char[] cbuf = new char[CHAR_BUF_SIZE];
/** block data mode */ private boolean blkmode = false; @@ -1763,6 +1765,18 @@ /** loopback stream (for data writes that span data blocks) */ private final DataOutputStream dout;
+ /** use unsafe to directly access value field in java.lang.String */ + private static final Unsafe unsafe = Unsafe.getUnsafe(); + /** use field offset to directly access value field in java.lang.String */ + private static final long stringValueOffset; + static { + try { + stringValueOffset = unsafe.objectFieldOffset(String.class.getDeclaredField("value")); + } catch (NoSuchFieldException e) { + throw new InternalError(e); + } + } + /** * Creates new BlockDataOutputStream on top of given underlying stream. * Block data mode is turned off by default. @@ -1972,35 +1986,23 @@ }
public void writeBytes(String s) throws IOException { - int endoff = s.length(); - int cpos = 0; - int csize = 0; + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + int endoff = sChars.length; for (int off = 0; off < endoff; ) { - if (cpos >= csize) { - cpos = 0; - csize = Math.min(endoff - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - } if (pos >= MAX_BLOCK_SIZE) { drain(); } - int n = Math.min(csize - cpos, MAX_BLOCK_SIZE - pos); + int n = Math.min(endoff - off, MAX_BLOCK_SIZE - pos); int stop = pos + n; while (pos < stop) { - buf[pos++] = (byte) cbuf[cpos++]; + buf[pos++] = (byte) sChars[off++]; } - off += n; } }
public void writeChars(String s) throws IOException { - int endoff = s.length(); - for (int off = 0; off < endoff; ) { - int csize = Math.min(endoff - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - writeChars(cbuf, 0, csize); - off += csize; - } + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + writeChars(sChars, 0, sChars.length); }
public void writeUTF(String s) throws IOException { @@ -2130,25 +2132,21 @@ }
/** - * Returns the length in bytes of the UTF encoding of the given string. + * Returns the length in bytes of the UTF encoding of this given string. */ - long getUTFLength(String s) { - int len = s.length(); + static long getUTFLength(String s) { + char[] value = (char[])unsafe.getObject(s, stringValueOffset); + int len = value.length; long utflen = 0; for (int off = 0; off < len; ) { - int csize = Math.min(len - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - for (int cpos = 0; cpos < csize; cpos++) { - char c = cbuf[cpos]; - if (c >= 0x0001 && c <= 0x007F) { - utflen++; - } else if (c > 0x07FF) { - utflen += 3; - } else { - utflen += 2; - } + char c = value[off++]; + if (c >= 0x0001 && c <= 0x007F) { + utflen++; + } else if (c > 0x07FF) { + utflen += 3; + } else { + utflen += 2; } - off += csize; } return utflen; } @@ -2198,40 +2196,36 @@ * 8-byte length header) of the UTF encoding for the given string. */ private void writeUTFBody(String s) throws IOException { + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + int len = sChars.length; int limit = MAX_BLOCK_SIZE - 3; - int len = s.length(); for (int off = 0; off < len; ) { - int csize = Math.min(len - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - for (int cpos = 0; cpos < csize; cpos++) { - char c = cbuf[cpos]; - if (pos <= limit) { - if (c <= 0x007F && c != 0) { - buf[pos++] = (byte) c; - } else if (c > 0x07FF) { - buf[pos + 2] = (byte) (0x80 | ((c >> 0) & 0x3F)); - buf[pos + 1] = (byte) (0x80 | ((c >> 6) & 0x3F)); - buf[pos + 0] = (byte) (0xE0 | ((c >> 12) & 0x0F)); - pos += 3; - } else { - buf[pos + 1] = (byte) (0x80 | ((c >> 0) & 0x3F)); - buf[pos + 0] = (byte) (0xC0 | ((c >> 6) & 0x1F)); - pos += 2; - } - } else { // write one byte at a time to normalize block - if (c <= 0x007F && c != 0) { - write(c); - } else if (c > 0x07FF) { - write(0xE0 | ((c >> 12) & 0x0F)); - write(0x80 | ((c >> 6) & 0x3F)); - write(0x80 | ((c >> 0) & 0x3F)); - } else { - write(0xC0 | ((c >> 6) & 0x1F)); - write(0x80 | ((c >> 0) & 0x3F)); - } + char c = sChars[off++]; + if (pos <= limit) { + if (c <= 0x007F && c != 0) { + buf[pos++] = (byte) c; + } else if (c > 0x07FF) { + buf[pos + 2] = (byte) (0x80 | ( c & 0x3F)); + buf[pos + 1] = (byte) (0x80 | ((c >> 6) & 0x3F)); + buf[pos ] = (byte) (0xE0 | ((c >> 12) & 0x0F)); + pos += 3; + } else { + buf[pos + 1] = (byte) (0x80 | ( c & 0x3F)); + buf[pos ] = (byte) (0xC0 | ((c >> 6) & 0x1F)); + pos += 2; + } + } else { // write one byte at a time to normalize block + if (c <= 0x007F && c != 0) { + write(c); + } else if (c > 0x07FF) { + write(0xE0 | ((c >> 12) & 0x0F)); + write(0x80 | ((c >> 6) & 0x3F)); + write(0x80 | ( c & 0x3F)); + } else { + write(0xC0 | ((c >> 6) & 0x1F)); + write(0x80 | ( c & 0x3F)); } } - off += csize; } } } @@ -2464,7 +2458,10 @@ StringBuilder buffer = new StringBuilder(); if (!stack.isEmpty()) { for(int i = stack.size(); i > 0; i-- ) { - buffer.append(stack.get(i-1) + ((i != 1) ? "\n" : "")); + buffer.append(stack.get(i - 1)); + if (i!=1) { + buffer.append('\n'); + } } } return buffer.toString(); diff --git a/src/share/classes/java/io/ObjectStreamClass.java b/src/share/classes/java/io/ObjectStreamClass.java --- a/src/share/classes/java/io/ObjectStreamClass.java +++ b/src/share/classes/java/io/ObjectStreamClass.java @@ -1474,7 +1474,28 @@ /** * Returns JVM type signature for given class. */ - private static String getClassSignature(Class<?> cl) { + static String getClassSignature(Class<?> cl) { + if (cl.isPrimitive()) + if (cl == Integer.TYPE) { + return "I"; + } else if (cl == Byte.TYPE) { + return "B"; + } else if (cl == Long.TYPE) { + return "J"; + } else if (cl == Float.TYPE) { + return "F"; + } else if (cl == Double.TYPE) { + return "D"; + } else if (cl == Short.TYPE) { + return "S"; + } else if (cl == Character.TYPE) { + return "C"; + } else if (cl == Boolean.TYPE) { + return "Z"; + } else if (cl == Void.TYPE) { + return "V"; + } + StringBuilder sbuf = new StringBuilder(); while (cl.isArray()) { sbuf.append('['); @@ -1503,7 +1524,7 @@ throw new InternalError(); } } else { - sbuf.append('L' + cl.getName().replace('.', '/') + ';'); + sbuf.append('L').append(cl.getName().replace('.', '/')).append(';'); } return sbuf.toString(); } diff --git a/src/share/classes/java/io/ObjectStreamField.java b/src/share/classes/java/io/ObjectStreamField.java --- a/src/share/classes/java/io/ObjectStreamField.java +++ b/src/share/classes/java/io/ObjectStreamField.java @@ -91,7 +91,7 @@ this.name = name; this.type = type; this.unshared = unshared; - signature = getClassSignature(type).intern(); + signature = ObjectStreamClass.getClassSignature(type).intern(); field = null; }
@@ -137,7 +137,7 @@ name = field.getName(); Class<?> ftype = field.getType(); type = (showType || ftype.isPrimitive()) ? ftype : Object.class; - signature = getClassSignature(ftype).intern(); + signature = ObjectStreamClass.getClassSignature(ftype).intern(); }
/** @@ -286,41 +286,4 @@ String getSignature() { return signature; } - - /** - * Returns JVM type signature for given class. - */ - private static String getClassSignature(Class<?> cl) { - StringBuilder sbuf = new StringBuilder(); - while (cl.isArray()) { - sbuf.append('['); - cl = cl.getComponentType(); - } - if (cl.isPrimitive()) { - if (cl == Integer.TYPE) { - sbuf.append('I'); - } else if (cl == Byte.TYPE) { - sbuf.append('B'); - } else if (cl == Long.TYPE) { - sbuf.append('J'); - } else if (cl == Float.TYPE) { - sbuf.append('F'); - } else if (cl == Double.TYPE) { - sbuf.append('D'); - } else if (cl == Short.TYPE) { - sbuf.append('S'); - } else if (cl == Character.TYPE) { - sbuf.append('C'); - } else if (cl == Boolean.TYPE) { - sbuf.append('Z'); - } else if (cl == Void.TYPE) { - sbuf.append('V'); - } else { - throw new InternalError(); - } - } else { - sbuf.append('L' + cl.getName().replace('.', '/') + ';'); - } - return sbuf.toString(); - } } *Gesendet:* Dienstag, 07. Januar 2014 um 10:05 Uhr *Von:* "Chris Hegarty" <chris.hegarty@oracle.com> *An:* "Robert Stupp" <snazy@gmx.de> *Cc:* "core-libs-dev@openjdk.java.net Libs" <core-libs-dev@openjdk.java.net> *Betreff:* Re: ObjectIn/OutputStream improvements On 15 Dec 2013, at 10:29, Robert Stupp <snazy@gmx.de> wrote:
Hi,
I digged through the object serialization code and found some lines that could be optimized to reduce the number of calls to System.arraycopy() and temporary object allocations especially during string (de)serialization. In short sentences the changes are: ObjectInputStream: - skip primitive/object reading if no primitive/objects in class (defaultReadFields method) - use shared StringBuilder for string reading (prevent superfluous object allocations of one StingBuilder and one implicit char[] for each string being deserialized) ObjectOutputStream: - skip primitive/object writing if no primitive/objects in class (defaultWriteFields method) - use unsafe access to calculate UTF-length - use unsafe access in readBytes() and writeChars() methods to access String value field - removed cbuf field ObjectStreamClass/ObjectStreamField: - minor improvement in getClassSignature ; share method code with ObjectStreamField (return constant string for primitives)
I have tested the changes in a big Java installation at a customer (backported the Java8 ObjectIn/OutputStream including the changes to Java6) and a long running real application performance test resulted in reduced CPU usage (from about 60% per server to 50%). The changes I made in openjdk8 pass all tests.
Since I have no experience how to contribute code to openjdk in form of a push/changeset I have added the diff (hg export -g) to this email.
Did you attach the diffs? I don’t see any attachment, it may be that the attachment was stripped. Can you try resending inline?
You should take a look at the OpenJDK How to Contribute page [1]. Paying particular attention to the OCA, without it we will not be able to move your patch forward.
Thanks, -Chris.
[1] http://openjdk.java.net/contribute/
Robert
On 1/31/14 10:46 AM, Chris Hegarty wrote:
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
I agree with splitting the Unsafe usages so they can be reviewed separately. New and changed usage of Unsafe will require exacting scrutiny. In general, the cleanups and refactorings look fine. I have a question about the change to reuse StringBuilder instances. This replaces freshly allocated StringBuilders stored in local variables with reuse of a StringBuilder stored in a field of BlockDataInputStream, which in turn is stored in a field of ObjectInputStream. Thus, instead of creating of lots of temporaries that become gc-eligible very quickly, this creates a single long-lived object whose size is the high-water mark of the longest string that's been read. The change does reduce allocation pressure, but the point of generational GC is to make allocation and collection of temporaries quite inexpensive. Is this the right tradeoff? s'marks
On 31 Jan 2014, at 19:07, Stuart Marks <stuart.marks@oracle.com> wrote:
On 1/31/14 10:46 AM, Chris Hegarty wrote:
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
I agree with splitting the Unsafe usages so they can be reviewed separately. New and changed usage of Unsafe will require exacting scrutiny.
In general, the cleanups and refactorings look fine.
I have a question about the change to reuse StringBuilder instances. This replaces freshly allocated StringBuilders stored in local variables with reuse of a StringBuilder stored in a field of BlockDataInputStream, which in turn is stored in a field of ObjectInputStream. Thus, instead of creating of lots of temporaries that become gc-eligible very quickly, this creates a single long-lived object whose size is the high-water mark of the longest string that's been read. The change does reduce allocation pressure, but the point of generational GC is to make allocation and collection of temporaries quite inexpensive. Is this the right tradeoff?
Good point Stuart. I can’t imagine that reusing is a problem, provided we can release. It may be that we should look at clearing the reference, and others, in close(). -Chris.
s'marks
One would have to measure of course, but intuitively, it seems like a good change to reuse the stringbuilder. There's also the issue that the local stringbuilder before was default-sized, and for large content, it would generate even more garbage as the underlying array is expanded. The "temporary short lived allocations are cheap" is, unfortunately, a semi-myth, or at least somewhat misguided. It's true that individually they may be cheap, but they have macro effects. The higher the allocation rate, the more young GCs we get. Every young GC (on hotspot collectors, at least) is a STW pause. Bringing threads to a safepoint has some cost, especially if there are many of them on large many-core machines. With larger heaps these days, young gens tend to be larger as well. When GC runs, it trashes the cpu caches for the mutators, so when they resume, they may get cache misses. At each young GC, some objects may get promoted prematurely to tenured. So no, I wouldn't say it's quite inexpensive :). When you have no option but to allocate, it's nice to have collectors that can handle those necessary allocations well. Otherwise, if it's a perf sensitive area and avoiding allocations doesn't obfuscate or make the code significantly less maintainable, it's usually better to avoid allocations. Just my $.02 Sent from my phone On Jan 31, 2014 2:06 PM, "Stuart Marks" <stuart.marks@oracle.com> wrote:
On 1/31/14 10:46 AM, Chris Hegarty wrote:
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
I agree with splitting the Unsafe usages so they can be reviewed separately. New and changed usage of Unsafe will require exacting scrutiny.
In general, the cleanups and refactorings look fine.
I have a question about the change to reuse StringBuilder instances. This replaces freshly allocated StringBuilders stored in local variables with reuse of a StringBuilder stored in a field of BlockDataInputStream, which in turn is stored in a field of ObjectInputStream. Thus, instead of creating of lots of temporaries that become gc-eligible very quickly, this creates a single long-lived object whose size is the high-water mark of the longest string that's been read. The change does reduce allocation pressure, but the point of generational GC is to make allocation and collection of temporaries quite inexpensive. Is this the right tradeoff?
s'marks
Nobody took the bait on this yet? :-) Certainly there's a lot of semi-myth on this topic, on both sides. Here's my source of mythology (or urban legend, as one might have it): http://www.ibm.com/developerworks/java/library/j-jtp09275/index.html My concern here is not so much about leaking of the StringBuilder held in a field, as Chris seemed to be responding to. I'd expect the ObjectInputStream to be GC'd at some point along with any StringBuilders it contains references to. I'm more concerned about defeating any optimizations that might occur if local variables are converted to fields. The article referenced above mentions escape analysis and possible stack allocation of locally-created objects. Offhand I don't know if stack allocation occurs for any of the builders in ObjectInputStream, but it certainly cannot occur if references are stored in fields. It would good if there were some evidence we could discuss instead of myths and urban legends. :-) Perhaps the original poster (Robert Stupp) can rerun some benchmarks with and without the conversion from locals to fields. (Earlier in the thread he posted some significant performance improvements, but my suspicion is that most of that improvement came from the conversion to use Unsafe.) I'm mindful that this may require a lot of effort. I think it would take a fair bit of work to come up with a benchmark that shows any difference between the two cases. I'm also mindful that one's intuition is often wrong. s'marks On 1/31/14 5:53 PM, Vitaly Davidovich wrote:
One would have to measure of course, but intuitively, it seems like a good change to reuse the stringbuilder. There's also the issue that the local stringbuilder before was default-sized, and for large content, it would generate even more garbage as the underlying array is expanded.
The "temporary short lived allocations are cheap" is, unfortunately, a semi-myth, or at least somewhat misguided. It's true that individually they may be cheap, but they have macro effects. The higher the allocation rate, the more young GCs we get. Every young GC (on hotspot collectors, at least) is a STW pause. Bringing threads to a safepoint has some cost, especially if there are many of them on large many-core machines. With larger heaps these days, young gens tend to be larger as well. When GC runs, it trashes the cpu caches for the mutators, so when they resume, they may get cache misses. At each young GC, some objects may get promoted prematurely to tenured.
So no, I wouldn't say it's quite inexpensive :). When you have no option but to allocate, it's nice to have collectors that can handle those necessary allocations well. Otherwise, if it's a perf sensitive area and avoiding allocations doesn't obfuscate or make the code significantly less maintainable, it's usually better to avoid allocations.
Just my $.02
Sent from my phone
On Jan 31, 2014 2:06 PM, "Stuart Marks" <stuart.marks@oracle.com <mailto:stuart.marks@oracle.com>> wrote:
On 1/31/14 10:46 AM, Chris Hegarty wrote:
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
http://cr.openjdk.java.net/~chegar/serial_stupp.00/webrev/ <http://cr.openjdk.java.net/%7Echegar/serial_stupp.00/webrev/>
I agree with splitting the Unsafe usages so they can be reviewed separately. New and changed usage of Unsafe will require exacting scrutiny.
In general, the cleanups and refactorings look fine.
I have a question about the change to reuse StringBuilder instances. This replaces freshly allocated StringBuilders stored in local variables with reuse of a StringBuilder stored in a field of BlockDataInputStream, which in turn is stored in a field of ObjectInputStream. Thus, instead of creating of lots of temporaries that become gc-eligible very quickly, this creates a single long-lived object whose size is the high-water mark of the longest string that's been read. The change does reduce allocation pressure, but the point of generational GC is to make allocation and collection of temporaries quite inexpensive. Is this the right tradeoff?
s'marks
On Feb 3, 2014, at 11:01 PM, Stuart Marks <stuart.marks@oracle.com> wrote:
Nobody took the bait on this yet? :-)
Certainly there's a lot of semi-myth on this topic, on both sides. Here's my source of mythology (or urban legend, as one might have it):
http://www.ibm.com/developerworks/java/library/j-jtp09275/index.html
My concern here is not so much about leaking of the StringBuilder held in a field, as Chris seemed to be responding to. I'd expect the ObjectInputStream to be GC'd at some point along with any StringBuilders it contains references to.
I'm more concerned about defeating any optimizations that might occur if local variables are converted to fields. The article referenced above mentions escape analysis and possible stack allocation of locally-created objects. Offhand I don't know if stack allocation occurs for any of the builders in ObjectInputStream, but it certainly cannot occur if references are stored in fields.
The caching of the StringBuffer seems reasonable point fix. (If one is worried about field access retain/expand the original method signatures so field access is hoisted outside of the loops.) However, if we want to focus on performance i think it better to change the UTF decoding algorithm to target small'ish ASCII strings (size < CHAR_BUF_SIZE) thereby avoiding the use of StringBuilder for presumed common cases. (IIRC a similar approach in a different area was proposed by Sherman for converting strings to lower/upper cases.) I think that would be a better way to spend our performance investigation budget. Thus, it might be better to remove the StringBuilder field from this patch and submit another one focusing on UTF decoding; the other changes in the patch look good. Paul.
It would good if there were some evidence we could discuss instead of myths and urban legends. :-) Perhaps the original poster (Robert Stupp) can rerun some benchmarks with and without the conversion from locals to fields. (Earlier in the thread he posted some significant performance improvements, but my suspicion is that most of that improvement came from the conversion to use Unsafe.)
I'm mindful that this may require a lot of effort. I think it would take a fair bit of work to come up with a benchmark that shows any difference between the two cases. I'm also mindful that one's intuition is often wrong.
s'marks
Seems like good changes. ObjectStreamClass.java:: - Why not have getClassSignature() return an interned string? (that's if interning is actually essential. Are we sure it's not just overhead?) On Jan 31 2014, at 10:46 , Chris Hegarty <chris.hegarty@oracle.com> wrote:
Forwarding to correct core-libs-dev list.
-Chris.
On 31 Jan 2014, at 14:22, Chris Hegarty <chris.hegarty@oracle.com> wrote:
Hi Robert,
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
http://cr.openjdk.java.net/~chegar/serial_stupp.00/webrev/
Specifically, * I think for clarify and readability SerialCallbackContext checkAndSetUsed() should be invoked directly. It makes it very clear what the intent is. * Skip primitive/object reading/writing if no primitives/objects in the stream/class. ( I personally don't see any benefit of rounding up the size of the array, it just seems to add unnecessary complexity ) * Move primitive types into getPrimitiveSignature for better reuse of code. This retains your change to not create the additional StringBuilder when it is not necessary.
I am currently running tests on this change.
-Chris.
On 07/01/14 13:03, Robert Stupp wrote:
Hi, I've attached the diff to the original email - it has been stripped. Here's a second try (inline). I've already signed the OCA and it has been approved :) Robert # HG changeset patch # User snazy # Date 1387101091 -3600 # Sun Dec 15 10:51:31 2013 +0100 # Node ID 6d46d99212453017c88417678d08dc8f10da9606 # Parent 9e1be800420265e5dcf264a7ed4abb6f230dd19d Removed some unnecessary variable assignments. ObjectInputStream: - skip primitive/object reading if no primitive/objects in class - use shared StringBuilder for string reading (prevent superfluous object allocations) ObjectOutputStream: - skip primitive/object writing if no primitive/objects in class - use unsafe access to calculate UTF-length - use unsafe access in readBytes() and writeChars() methods to access String value field - removed cbuf field ObjectStreamClass/ObjectStreamField: - minor improvement in getClassSignature ; share method code with ObjectStreamField diff --git a/src/share/classes/java/io/ObjectInputStream.java b/src/share/classes/java/io/ObjectInputStream.java --- a/src/share/classes/java/io/ObjectInputStream.java +++ b/src/share/classes/java/io/ObjectInputStream.java @@ -39,8 +39,8 @@ import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; import static java.io.ObjectStreamClass.processQueue; + import sun.reflect.misc.ReflectUtil;
/** @@ -534,7 +534,7 @@ if (ctx == null) { throw new NotActiveException("not in call to readObject"); } - Object curObj = ctx.getObj(); + ctx.getObj(); ObjectStreamClass curDesc = ctx.getDesc(); bin.setBlockDataMode(false); GetFieldImpl getField = new GetFieldImpl(curDesc); @@ -1597,7 +1597,7 @@ int descHandle = handles.assign(unshared ? unsharedMarker : desc); passHandle = NULL_HANDLE;
- ObjectStreamClass readDesc = null; + ObjectStreamClass readDesc; try { readDesc = readClassDescriptor(); } catch (ClassNotFoundException ex) { @@ -1976,29 +1976,34 @@ }
int primDataSize = desc.getPrimDataSize(); - if (primVals == null || primVals.length < primDataSize) { - primVals = new byte[primDataSize]; - } - bin.readFully(primVals, 0, primDataSize, false); - if (obj != null) { - desc.setPrimFieldValues(obj, primVals); - } - - int objHandle = passHandle; - ObjectStreamField[] fields = desc.getFields(false); - Object[] objVals = new Object[desc.getNumObjFields()]; - int numPrimFields = fields.length - objVals.length; - for (int i = 0; i < objVals.length; i++) { - ObjectStreamField f = fields[numPrimFields + i]; - objVals[i] = readObject0(f.isUnshared()); - if (f.getField() != null) { - handles.markDependency(objHandle, passHandle); + if (primDataSize > 0) { + if (primVals == null || primVals.length < primDataSize) { + primVals = new byte[ ((primDataSize>>4)+1)<<4 ]; + } + bin.readFully(primVals, 0, primDataSize, false); + if (obj != null) { + desc.setPrimFieldValues(obj, primVals); } } - if (obj != null) { - desc.setObjFieldValues(obj, objVals); + + int numObjFields = desc.getNumObjFields(); + if (numObjFields > 0) { + int objHandle = passHandle; + ObjectStreamField[] fields = desc.getFields(false); + Object[] objVals = new Object[numObjFields]; + int numPrimFields = fields.length - objVals.length; + for (int i = 0; i < objVals.length; i++) { + ObjectStreamField f = fields[numPrimFields + i]; + objVals[i] = readObject0(f.isUnshared()); + if (f.getField() != null) { + handles.markDependency(objHandle, passHandle); + } + } + if (obj != null) { + desc.setObjFieldValues(obj, objVals); + } + passHandle = objHandle; } - passHandle = objHandle; }
/** @@ -2377,8 +2382,10 @@ private final byte[] buf = new byte[MAX_BLOCK_SIZE]; /** buffer for reading block data headers */ private final byte[] hbuf = new byte[MAX_HEADER_SIZE]; - /** char buffer for fast string reads */ + /** char buffer for fast string reads - used by {@link #readUTFSpan(long)} */ private final char[] cbuf = new char[CHAR_BUF_SIZE]; + /** shared string builder for less object allocations - used by {@link #readUTFBody(long)}, {@link #readUTFChar(long)} and {@link #readUTFSpan(long)} */ + private final StringBuilder sbuf = new StringBuilder(CHAR_BUF_SIZE);
/** block data mode */ private boolean blkmode = false; @@ -3044,19 +3051,19 @@ * utflen bytes. */ private String readUTFBody(long utflen) throws IOException { - StringBuilder sbuf = new StringBuilder(); if (!blkmode) { end = pos = 0; }
+ sbuf.setLength(0); while (utflen > 0) { int avail = end - pos; if (avail >= 3 || (long) avail == utflen) { - utflen -= readUTFSpan(sbuf, utflen); + utflen -= readUTFSpan(utflen); } else { if (blkmode) { // near block boundary, read one byte at a time - utflen -= readUTFChar(sbuf, utflen); + utflen -= readUTFChar(utflen); } else { // shift and refill buffer manually if (avail > 0) { @@ -3076,10 +3083,10 @@ * Reads span of UTF-encoded characters out of internal buffer * (starting at offset pos and ending at or before offset end), * consuming no more than utflen bytes. Appends read characters to - * sbuf. Returns the number of bytes consumed. + * {@link #sbuf}. Returns the number of bytes consumed. */ - private long readUTFSpan(StringBuilder sbuf, long utflen) - throws IOException + private long readUTFSpan(long utflen) + throws IOException { int cpos = 0; int start = pos; @@ -3111,19 +3118,19 @@ throw new UTFDataFormatException(); } cbuf[cpos++] = (char) (((b1 & 0x1F) << 6) | - ((b2 & 0x3F) << 0)); + (b2 & 0x3F)); break;
case 14: // 3 byte format: 1110xxxx 10xxxxxx 10xxxxxx b3 = buf[pos + 1]; - b2 = buf[pos + 0]; + b2 = buf[pos ]; pos += 2; if ((b2 & 0xC0) != 0x80 || (b3 & 0xC0) != 0x80) { throw new UTFDataFormatException(); } cbuf[cpos++] = (char) (((b1 & 0x0F) << 12) | - ((b2 & 0x3F) << 6) | - ((b3 & 0x3F) << 0)); + ((b2 & 0x3F) << 6) | + (b3 & 0x3F)); break;
default: // 10xx xxxx, 1111 xxxx @@ -3150,12 +3157,12 @@
/** * Reads in single UTF-encoded character one byte at a time, appends - * the character to sbuf, and returns the number of bytes consumed. + * the character to {@link #sbuf}, and returns the number of bytes consumed. * This method is used when reading in UTF strings written in block * data mode to handle UTF-encoded characters which (potentially) * straddle block-data boundaries. */ - private int readUTFChar(StringBuilder sbuf, long utflen) + private int readUTFChar(long utflen) throws IOException { int b1, b2, b3; @@ -3181,8 +3188,7 @@ if ((b2 & 0xC0) != 0x80) { throw new UTFDataFormatException(); } - sbuf.append((char) (((b1 & 0x1F) << 6) | - ((b2 & 0x3F) << 0))); + sbuf.append((char) (((b1 & 0x1F) << 6) | (b2 & 0x3F))); return 2;
case 14: // 3 byte format: 1110xxxx 10xxxxxx 10xxxxxx @@ -3198,8 +3204,8 @@ throw new UTFDataFormatException(); } sbuf.append((char) (((b1 & 0x0F) << 12) | - ((b2 & 0x3F) << 6) | - ((b3 & 0x3F) << 0))); + ((b2 & 0x3F) << 6) | + (b3 & 0x3F))); return 3;
default: // 10xx xxxx, 1111 xxxx diff --git a/src/share/classes/java/io/ObjectOutputStream.java b/src/share/classes/java/io/ObjectOutputStream.java --- a/src/share/classes/java/io/ObjectOutputStream.java +++ b/src/share/classes/java/io/ObjectOutputStream.java @@ -35,7 +35,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import static java.io.ObjectStreamClass.processQueue; -import java.io.SerialCallbackContext; + +import sun.misc.Unsafe; import sun.reflect.misc.ReflectUtil;
/** @@ -458,7 +459,7 @@ if (ctx == null) { throw new NotActiveException("not in call to writeObject"); } - Object curObj = ctx.getObj(); + ctx.getObj(); ObjectStreamClass curDesc = ctx.getDesc(); curPut = new PutFieldImpl(curDesc); } @@ -1300,7 +1301,7 @@ */ private void writeString(String str, boolean unshared) throws IOException { handles.assign(unshared ? null : str); - long utflen = bout.getUTFLength(str); + long utflen = BlockDataOutputStream.getUTFLength(str); if (utflen <= 0xFFFF) { bout.writeByte(TC_STRING); bout.writeUTF(str, utflen); @@ -1527,29 +1528,34 @@ desc.checkDefaultSerialize();
int primDataSize = desc.getPrimDataSize(); - if (primVals == null || primVals.length < primDataSize) { - primVals = new byte[primDataSize]; + if (primDataSize > 0) { + if (primVals == null || primVals.length < primDataSize) { + primVals = new byte[ ((primDataSize>>4)+1)<<4 ]; + } + desc.getPrimFieldValues(obj, primVals); + bout.write(primVals, 0, primDataSize, false); } - desc.getPrimFieldValues(obj, primVals); - bout.write(primVals, 0, primDataSize, false);
- ObjectStreamField[] fields = desc.getFields(false); - Object[] objVals = new Object[desc.getNumObjFields()]; - int numPrimFields = fields.length - objVals.length; - desc.getObjFieldValues(obj, objVals); - for (int i = 0; i < objVals.length; i++) { - if (extendedDebugInfo) { - debugInfoStack.push( - "field (class \"" + desc.getName() + "\", name: \"" + - fields[numPrimFields + i].getName() + "\", type: \"" + - fields[numPrimFields + i].getType() + "\")"); - } - try { - writeObject0(objVals[i], - fields[numPrimFields + i].isUnshared()); - } finally { + int numObjFields = desc.getNumObjFields(); + if (numObjFields > 0) { + ObjectStreamField[] fields = desc.getFields(false); + Object[] objVals = new Object[numObjFields]; + int numPrimFields = fields.length - objVals.length; + desc.getObjFieldValues(obj, objVals); + for (int i = 0; i < objVals.length; i++) { if (extendedDebugInfo) { - debugInfoStack.pop(); + debugInfoStack.push( + "field (class \"" + desc.getName() + "\", name: \"" + + fields[numPrimFields + i].getName() + "\", type: \"" + + fields[numPrimFields + i].getType() + "\")"); + } + try { + writeObject0(objVals[i], + fields[numPrimFields + i].isUnshared()); + } finally { + if (extendedDebugInfo) { + debugInfoStack.pop(); + } } } } @@ -1743,15 +1749,11 @@ private static final int MAX_BLOCK_SIZE = 1024; /** maximum data block header length */ private static final int MAX_HEADER_SIZE = 5; - /** (tunable) length of char buffer (for writing strings) */ - private static final int CHAR_BUF_SIZE = 256;
/** buffer for writing general/block data */ private final byte[] buf = new byte[MAX_BLOCK_SIZE]; /** buffer for writing block data headers */ private final byte[] hbuf = new byte[MAX_HEADER_SIZE]; - /** char buffer for fast string writes */ - private final char[] cbuf = new char[CHAR_BUF_SIZE];
/** block data mode */ private boolean blkmode = false; @@ -1763,6 +1765,18 @@ /** loopback stream (for data writes that span data blocks) */ private final DataOutputStream dout;
+ /** use unsafe to directly access value field in java.lang.String */ + private static final Unsafe unsafe = Unsafe.getUnsafe(); + /** use field offset to directly access value field in java.lang.String */ + private static final long stringValueOffset; + static { + try { + stringValueOffset = unsafe.objectFieldOffset(String.class.getDeclaredField("value")); + } catch (NoSuchFieldException e) { + throw new InternalError(e); + } + } + /** * Creates new BlockDataOutputStream on top of given underlying stream. * Block data mode is turned off by default. @@ -1972,35 +1986,23 @@ }
public void writeBytes(String s) throws IOException { - int endoff = s.length(); - int cpos = 0; - int csize = 0; + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + int endoff = sChars.length; for (int off = 0; off < endoff; ) { - if (cpos >= csize) { - cpos = 0; - csize = Math.min(endoff - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - } if (pos >= MAX_BLOCK_SIZE) { drain(); } - int n = Math.min(csize - cpos, MAX_BLOCK_SIZE - pos); + int n = Math.min(endoff - off, MAX_BLOCK_SIZE - pos); int stop = pos + n; while (pos < stop) { - buf[pos++] = (byte) cbuf[cpos++]; + buf[pos++] = (byte) sChars[off++]; } - off += n; } }
public void writeChars(String s) throws IOException { - int endoff = s.length(); - for (int off = 0; off < endoff; ) { - int csize = Math.min(endoff - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - writeChars(cbuf, 0, csize); - off += csize; - } + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + writeChars(sChars, 0, sChars.length); }
public void writeUTF(String s) throws IOException { @@ -2130,25 +2132,21 @@ }
/** - * Returns the length in bytes of the UTF encoding of the given string. + * Returns the length in bytes of the UTF encoding of this given string. */ - long getUTFLength(String s) { - int len = s.length(); + static long getUTFLength(String s) { + char[] value = (char[])unsafe.getObject(s, stringValueOffset); + int len = value.length; long utflen = 0; for (int off = 0; off < len; ) { - int csize = Math.min(len - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - for (int cpos = 0; cpos < csize; cpos++) { - char c = cbuf[cpos]; - if (c >= 0x0001 && c <= 0x007F) { - utflen++; - } else if (c > 0x07FF) { - utflen += 3; - } else { - utflen += 2; - } + char c = value[off++]; + if (c >= 0x0001 && c <= 0x007F) { + utflen++; + } else if (c > 0x07FF) { + utflen += 3; + } else { + utflen += 2; } - off += csize; } return utflen; } @@ -2198,40 +2196,36 @@ * 8-byte length header) of the UTF encoding for the given string. */ private void writeUTFBody(String s) throws IOException { + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + int len = sChars.length; int limit = MAX_BLOCK_SIZE - 3; - int len = s.length(); for (int off = 0; off < len; ) { - int csize = Math.min(len - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - for (int cpos = 0; cpos < csize; cpos++) { - char c = cbuf[cpos]; - if (pos <= limit) { - if (c <= 0x007F && c != 0) { - buf[pos++] = (byte) c; - } else if (c > 0x07FF) { - buf[pos + 2] = (byte) (0x80 | ((c >> 0) & 0x3F)); - buf[pos + 1] = (byte) (0x80 | ((c >> 6) & 0x3F)); - buf[pos + 0] = (byte) (0xE0 | ((c >> 12) & 0x0F)); - pos += 3; - } else { - buf[pos + 1] = (byte) (0x80 | ((c >> 0) & 0x3F)); - buf[pos + 0] = (byte) (0xC0 | ((c >> 6) & 0x1F)); - pos += 2; - } - } else { // write one byte at a time to normalize block - if (c <= 0x007F && c != 0) { - write(c); - } else if (c > 0x07FF) { - write(0xE0 | ((c >> 12) & 0x0F)); - write(0x80 | ((c >> 6) & 0x3F)); - write(0x80 | ((c >> 0) & 0x3F)); - } else { - write(0xC0 | ((c >> 6) & 0x1F)); - write(0x80 | ((c >> 0) & 0x3F)); - } + char c = sChars[off++]; + if (pos <= limit) { + if (c <= 0x007F && c != 0) { + buf[pos++] = (byte) c; + } else if (c > 0x07FF) { + buf[pos + 2] = (byte) (0x80 | ( c & 0x3F)); + buf[pos + 1] = (byte) (0x80 | ((c >> 6) & 0x3F)); + buf[pos ] = (byte) (0xE0 | ((c >> 12) & 0x0F)); + pos += 3; + } else { + buf[pos + 1] = (byte) (0x80 | ( c & 0x3F)); + buf[pos ] = (byte) (0xC0 | ((c >> 6) & 0x1F)); + pos += 2; + } + } else { // write one byte at a time to normalize block + if (c <= 0x007F && c != 0) { + write(c); + } else if (c > 0x07FF) { + write(0xE0 | ((c >> 12) & 0x0F)); + write(0x80 | ((c >> 6) & 0x3F)); + write(0x80 | ( c & 0x3F)); + } else { + write(0xC0 | ((c >> 6) & 0x1F)); + write(0x80 | ( c & 0x3F)); } } - off += csize; } } } @@ -2464,7 +2458,10 @@ StringBuilder buffer = new StringBuilder(); if (!stack.isEmpty()) { for(int i = stack.size(); i > 0; i-- ) { - buffer.append(stack.get(i-1) + ((i != 1) ? "\n" : "")); + buffer.append(stack.get(i - 1)); + if (i!=1) { + buffer.append('\n'); + } } } return buffer.toString(); diff --git a/src/share/classes/java/io/ObjectStreamClass.java b/src/share/classes/java/io/ObjectStreamClass.java --- a/src/share/classes/java/io/ObjectStreamClass.java +++ b/src/share/classes/java/io/ObjectStreamClass.java @@ -1474,7 +1474,28 @@ /** * Returns JVM type signature for given class. */ - private static String getClassSignature(Class<?> cl) { + static String getClassSignature(Class<?> cl) { + if (cl.isPrimitive()) + if (cl == Integer.TYPE) { + return "I"; + } else if (cl == Byte.TYPE) { + return "B"; + } else if (cl == Long.TYPE) { + return "J"; + } else if (cl == Float.TYPE) { + return "F"; + } else if (cl == Double.TYPE) { + return "D"; + } else if (cl == Short.TYPE) { + return "S"; + } else if (cl == Character.TYPE) { + return "C"; + } else if (cl == Boolean.TYPE) { + return "Z"; + } else if (cl == Void.TYPE) { + return "V"; + } + StringBuilder sbuf = new StringBuilder(); while (cl.isArray()) { sbuf.append('['); @@ -1503,7 +1524,7 @@ throw new InternalError(); } } else { - sbuf.append('L' + cl.getName().replace('.', '/') + ';'); + sbuf.append('L').append(cl.getName().replace('.', '/')).append(';'); } return sbuf.toString(); } diff --git a/src/share/classes/java/io/ObjectStreamField.java b/src/share/classes/java/io/ObjectStreamField.java --- a/src/share/classes/java/io/ObjectStreamField.java +++ b/src/share/classes/java/io/ObjectStreamField.java @@ -91,7 +91,7 @@ this.name = name; this.type = type; this.unshared = unshared; - signature = getClassSignature(type).intern(); + signature = ObjectStreamClass.getClassSignature(type).intern(); field = null; }
@@ -137,7 +137,7 @@ name = field.getName(); Class<?> ftype = field.getType(); type = (showType || ftype.isPrimitive()) ? ftype : Object.class; - signature = getClassSignature(ftype).intern(); + signature = ObjectStreamClass.getClassSignature(ftype).intern(); }
/** @@ -286,41 +286,4 @@ String getSignature() { return signature; } - - /** - * Returns JVM type signature for given class. - */ - private static String getClassSignature(Class<?> cl) { - StringBuilder sbuf = new StringBuilder(); - while (cl.isArray()) { - sbuf.append('['); - cl = cl.getComponentType(); - } - if (cl.isPrimitive()) { - if (cl == Integer.TYPE) { - sbuf.append('I'); - } else if (cl == Byte.TYPE) { - sbuf.append('B'); - } else if (cl == Long.TYPE) { - sbuf.append('J'); - } else if (cl == Float.TYPE) { - sbuf.append('F'); - } else if (cl == Double.TYPE) { - sbuf.append('D'); - } else if (cl == Short.TYPE) { - sbuf.append('S'); - } else if (cl == Character.TYPE) { - sbuf.append('C'); - } else if (cl == Boolean.TYPE) { - sbuf.append('Z'); - } else if (cl == Void.TYPE) { - sbuf.append('V'); - } else { - throw new InternalError(); - } - } else { - sbuf.append('L' + cl.getName().replace('.', '/') + ';'); - } - return sbuf.toString(); - } } *Gesendet:* Dienstag, 07. Januar 2014 um 10:05 Uhr *Von:* "Chris Hegarty" <chris.hegarty@oracle.com> *An:* "Robert Stupp" <snazy@gmx.de> *Cc:* "core-libs-dev@openjdk.java.net Libs" <core-libs-dev@openjdk.java.net> *Betreff:* Re: ObjectIn/OutputStream improvements On 15 Dec 2013, at 10:29, Robert Stupp <snazy@gmx.de> wrote:
Hi,
I digged through the object serialization code and found some lines that could be optimized to reduce the number of calls to System.arraycopy() and temporary object allocations especially during string (de)serialization. In short sentences the changes are: ObjectInputStream: - skip primitive/object reading if no primitive/objects in class (defaultReadFields method) - use shared StringBuilder for string reading (prevent superfluous object allocations of one StingBuilder and one implicit char[] for each string being deserialized) ObjectOutputStream: - skip primitive/object writing if no primitive/objects in class (defaultWriteFields method) - use unsafe access to calculate UTF-length - use unsafe access in readBytes() and writeChars() methods to access String value field - removed cbuf field ObjectStreamClass/ObjectStreamField: - minor improvement in getClassSignature ; share method code with ObjectStreamField (return constant string for primitives)
I have tested the changes in a big Java installation at a customer (backported the Java8 ObjectIn/OutputStream including the changes to Java6) and a long running real application performance test resulted in reduced CPU usage (from about 60% per server to 50%). The changes I made in openjdk8 pass all tests.
Since I have no experience how to contribute code to openjdk in form of a push/changeset I have added the diff (hg export -g) to this email.
Did you attach the diffs? I don’t see any attachment, it may be that the attachment was stripped. Can you try resending inline?
You should take a look at the OpenJDK How to Contribute page [1]. Paying particular attention to the OCA, without it we will not be able to move your patch forward.
Thanks, -Chris.
[1] http://openjdk.java.net/contribute/
Robert
Thanks stuart, Mike, and Paul.
- Why not have getClassSignature() return an interned string? (that's if interning is actually essential. Are we sure it's not just overhead?)
I didn’t want to change the existing use of interning here, just refactor the code a little to make it cleaner.
I think that would be a better way to spend our performance investigation budget. Thus, it might be better to remove the StringBuilder field from this patch and submit another one focusing on UTF decoding; the other changes in the patch look good.
Agreed. This could be looked at separately. Latest webrev: http://cr.openjdk.java.net/~chegar/serial_stupp.01/ Thanks, -Chris.
On 02/05/2014 04:11 PM, Chris Hegarty wrote:
Thanks stuart, Mike, and Paul.
- Why not have getClassSignature() return an interned string? (that's if interning is actually essential. Are we sure it's not just overhead?) I didn’t want to change the existing use of interning here, just refactor the code a little to make it cleaner.
I think that would be a better way to spend our performance investigation budget. Thus, it might be better to remove the StringBuilder field from this patch and submit another one focusing on UTF decoding; the other changes in the patch look good. Agreed. This could be looked at separately.
Latest webrev: http://cr.openjdk.java.net/~chegar/serial_stupp.01/
Thanks, -Chris.
Hi Chris, What about the following even less garbage-producing-and-copying ObjectStreamClass.get[Class|Method]Signature combo: /** * Returns JVM type signature for given class. */ static String getClassSignature(Class<?> cl) { if (cl.isPrimitive()) return getPrimitiveSignature(cl); else return appendClassSignature(new StringBuilder(), cl).toString(); } private static StringBuilder appendClassSignature(StringBuilder sbuf, Class<?> cl) { while (cl.isArray()) { sbuf.append('['); cl = cl.getComponentType(); } if (cl.isPrimitive()) sbuf.append(getPrimitiveSignature(cl)); else sbuf.append('L').append(cl.getName().replace('.', '/')).append(';'); return sbuf; } /** * Returns JVM type signature for given list of parameters and return type. */ private static String getMethodSignature(Class<?>[] paramTypes, Class<?> retType) { StringBuilder sbuf = new StringBuilder(); sbuf.append('('); for (int i = 0; i < paramTypes.length; i++) { appendClassSignature(sbuf, paramTypes[i]); } sbuf.append(')'); appendClassSignature(sbuf, retType); return sbuf.toString(); } Regards, Peter
Thanks Peter, this is a nice improvement. I’ll incorporate your changes before pushing. -Chris. On 5 Feb 2014, at 16:39, Peter Levart <peter.levart@gmail.com> wrote:
On 02/05/2014 04:11 PM, Chris Hegarty wrote:
Thanks stuart, Mike, and Paul.
- Why not have getClassSignature() return an interned string? (that's if interning is actually essential. Are we sure it's not just overhead?) I didn’t want to change the existing use of interning here, just refactor the code a little to make it cleaner.
I think that would be a better way to spend our performance investigation budget. Thus, it might be better to remove the StringBuilder field from this patch and submit another one focusing on UTF decoding; the other changes in the patch look good. Agreed. This could be looked at separately.
Latest webrev: http://cr.openjdk.java.net/~chegar/serial_stupp.01/
Thanks, -Chris.
Hi Chris,
What about the following even less garbage-producing-and-copying ObjectStreamClass.get[Class|Method]Signature combo:
/** * Returns JVM type signature for given class. */ static String getClassSignature(Class<?> cl) { if (cl.isPrimitive()) return getPrimitiveSignature(cl); else return appendClassSignature(new StringBuilder(), cl).toString(); }
private static StringBuilder appendClassSignature(StringBuilder sbuf, Class<?> cl) { while (cl.isArray()) { sbuf.append('['); cl = cl.getComponentType(); }
if (cl.isPrimitive()) sbuf.append(getPrimitiveSignature(cl)); else sbuf.append('L').append(cl.getName().replace('.', '/')).append(';');
return sbuf; }
/** * Returns JVM type signature for given list of parameters and return type. */ private static String getMethodSignature(Class<?>[] paramTypes, Class<?> retType) { StringBuilder sbuf = new StringBuilder(); sbuf.append('('); for (int i = 0; i < paramTypes.length; i++) { appendClassSignature(sbuf, paramTypes[i]); } sbuf.append(')'); appendClassSignature(sbuf, retType); return sbuf.toString(); }
Regards, Peter
Hi, I looked into the UTF serialization and deserialization code - and compared it a bit with the code behind "new String(byte[], Charset)", "String.getBytes(Charset)". Just to find something that can be safely reused in Object*Stream classes to optimize String handling. The first thing I noticed is that the class sun.nio.cs.UTF_8 class uses one byte for (char)0 - Object*Stream use two 0 bytes to represent (char)0. Although this representation is incompatible to the representation that the class UTF_8 uses it is not a big issue. The other thing is that Object*Stream seem not to be able to (de)serialize 21 bit characters. Methods in UTF_8 class are sun.nio.cs.UTF_8.Decoder#decode and sun.nio.cs.UTF_8.Encoder#encode. Is it ok to add 21-bit UTF-8 representation in Object*Stream? - Robert Am 06.02.2014 um 13:07 schrieb Chris Hegarty <chris.hegarty@oracle.com>:
Thanks Peter, this is a nice improvement. I’ll incorporate your changes before pushing.
-Chris.
On 5 Feb 2014, at 16:39, Peter Levart <peter.levart@gmail.com> wrote:
On 02/05/2014 04:11 PM, Chris Hegarty wrote:
Thanks stuart, Mike, and Paul.
- Why not have getClassSignature() return an interned string? (that's if interning is actually essential. Are we sure it's not just overhead?) I didn’t want to change the existing use of interning here, just refactor the code a little to make it cleaner.
I think that would be a better way to spend our performance investigation budget. Thus, it might be better to remove the StringBuilder field from this patch and submit another one focusing on UTF decoding; the other changes in the patch look good. Agreed. This could be looked at separately.
Latest webrev: http://cr.openjdk.java.net/~chegar/serial_stupp.01/
Thanks, -Chris.
Hi Chris,
What about the following even less garbage-producing-and-copying ObjectStreamClass.get[Class|Method]Signature combo:
/** * Returns JVM type signature for given class. */ static String getClassSignature(Class<?> cl) { if (cl.isPrimitive()) return getPrimitiveSignature(cl); else return appendClassSignature(new StringBuilder(), cl).toString(); }
private static StringBuilder appendClassSignature(StringBuilder sbuf, Class<?> cl) { while (cl.isArray()) { sbuf.append('['); cl = cl.getComponentType(); }
if (cl.isPrimitive()) sbuf.append(getPrimitiveSignature(cl)); else sbuf.append('L').append(cl.getName().replace('.', '/')).append(';');
return sbuf; }
/** * Returns JVM type signature for given list of parameters and return type. */ private static String getMethodSignature(Class<?>[] paramTypes, Class<?> retType) { StringBuilder sbuf = new StringBuilder(); sbuf.append('('); for (int i = 0; i < paramTypes.length; i++) { appendClassSignature(sbuf, paramTypes[i]); } sbuf.append(')'); appendClassSignature(sbuf, retType); return sbuf.toString(); }
Regards, Peter
I had forgotten about the use of Modified UTF-8 in serialization. Leave it alone. http://en.wikipedia.org/wiki/UTF-8#Modified_UTF-8 On Thu, Feb 6, 2014 at 2:50 PM, Robert Stupp <snazy@snazy.de> wrote:
Hi,
I looked into the UTF serialization and deserialization code - and compared it a bit with the code behind "new String(byte[], Charset)", "String.getBytes(Charset)". Just to find something that can be safely reused in Object*Stream classes to optimize String handling.
The first thing I noticed is that the class sun.nio.cs.UTF_8 class uses one byte for (char)0 - Object*Stream use two 0 bytes to represent (char)0. Although this representation is incompatible to the representation that the class UTF_8 uses it is not a big issue. The other thing is that Object*Stream seem not to be able to (de)serialize 21 bit characters. Methods in UTF_8 class are sun.nio.cs.UTF_8.Decoder#decode and sun.nio.cs.UTF_8.Encoder#encode. Is it ok to add 21-bit UTF-8 representation in Object*Stream?
- Robert
Am 06.02.2014 um 13:07 schrieb Chris Hegarty <chris.hegarty@oracle.com>:
Thanks Peter, this is a nice improvement. I’ll incorporate your changes before pushing.
-Chris.
On 5 Feb 2014, at 16:39, Peter Levart <peter.levart@gmail.com> wrote:
On 02/05/2014 04:11 PM, Chris Hegarty wrote:
Thanks stuart, Mike, and Paul.
- Why not have getClassSignature() return an interned string? (that's if interning is actually essential. Are we sure it's not just overhead?) I didn’t want to change the existing use of interning here, just refactor the code a little to make it cleaner.
I think that would be a better way to spend our performance investigation budget. Thus, it might be better to remove the StringBuilder field from this patch and submit another one focusing on UTF decoding; the other changes in the patch look good. Agreed. This could be looked at separately.
Latest webrev: http://cr.openjdk.java.net/~chegar/serial_stupp.01/
Thanks, -Chris.
Hi Chris,
What about the following even less garbage-producing-and-copying ObjectStreamClass.get[Class|Method]Signature combo:
/** * Returns JVM type signature for given class. */ static String getClassSignature(Class<?> cl) { if (cl.isPrimitive()) return getPrimitiveSignature(cl); else return appendClassSignature(new StringBuilder(), cl).toString(); }
private static StringBuilder appendClassSignature(StringBuilder sbuf, Class<?> cl) { while (cl.isArray()) { sbuf.append('['); cl = cl.getComponentType(); }
if (cl.isPrimitive()) sbuf.append(getPrimitiveSignature(cl)); else sbuf.append('L').append(cl.getName().replace('.', '/')).append(';');
return sbuf; }
/** * Returns JVM type signature for given list of parameters and return type. */ private static String getMethodSignature(Class<?>[] paramTypes, Class<?> retType) { StringBuilder sbuf = new StringBuilder(); sbuf.append('('); for (int i = 0; i < paramTypes.length; i++) { appendClassSignature(sbuf, paramTypes[i]); } sbuf.append(')'); appendClassSignature(sbuf, retType); return sbuf.toString(); }
Regards, Peter
eh - yes - ouch! 21 bit cannot occur (char = 16 bits) please, please ignore my comment about it. Am 07.02.2014 um 00:15 schrieb Xueming Shen <xueming.shen@oracle.com>:
Modified UTF8 has to be used, as specified by the spec.
http://download.java.net/jdk8/docs/api/java/io/DataInput.html#modified-utf-8
-Sherman
On 02/06/2014 02:50 PM, Robert Stupp wrote:
-Chris.
High-level thoughts: Although we all love garbage collection, performance sensitive code should still avoid unnecessary allocation. Escape analysis has not fulfilled its promise, and seems unlikely to in the future. If and when value types/tuples come to Java, we can create such objects with hopefully more confidence that they will be stack allocated in the common case. Memory management continues to be the biggest concern of people managing large Java applications. It continues to be true that Java applications need much more memory than "equivalent" C++ applications, and much more "memory tuning". OTOH, CPU usage parity is much less of an issue. --- I have done some (incomplete) work on making faster UTF-8 implementations, both inside and outside the JDK, some of which could be brought into the JDK proper and built upon. Increasingly, UTF-8 is the only important charset worth optimizing for. http://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/... http://code.google.com/p/protobuf/source/browse/trunk/java/src/main/java/com... On Fri, Jan 31, 2014 at 10:46 AM, Chris Hegarty <chris.hegarty@oracle.com>wrote:
Forwarding to correct core-libs-dev list.
-Chris.
On 31 Jan 2014, at 14:22, Chris Hegarty <chris.hegarty@oracle.com> wrote:
Hi Robert,
I think your patch can be split into two logical, independent, parts. The first is the use of unsafe to access the String UTF length. The seconds is to reuse, where possible, the existing StringBuilder instances, skipping of primitive/object reading/writing where applicable, and general cleanup.
Since this is a very sensitive area I would like to consider these separately. To that end, I have taken the changes that are applicable to the latter, removed any subjective stylistic changes, and made some additional cleanup improvements.
http://cr.openjdk.java.net/~chegar/serial_stupp.00/webrev/
Specifically, * I think for clarify and readability SerialCallbackContext checkAndSetUsed() should be invoked directly. It makes it very clear what the intent is. * Skip primitive/object reading/writing if no primitives/objects in the stream/class. ( I personally don't see any benefit of rounding up the size of the array, it just seems to add unnecessary complexity ) * Move primitive types into getPrimitiveSignature for better reuse of code. This retains your change to not create the additional StringBuilder when it is not necessary.
I am currently running tests on this change.
-Chris.
On 07/01/14 13:03, Robert Stupp wrote:
Hi, I've attached the diff to the original email - it has been stripped. Here's a second try (inline). I've already signed the OCA and it has been approved :) Robert # HG changeset patch # User snazy # Date 1387101091 -3600 # Sun Dec 15 10:51:31 2013 +0100 # Node ID 6d46d99212453017c88417678d08dc8f10da9606 # Parent 9e1be800420265e5dcf264a7ed4abb6f230dd19d Removed some unnecessary variable assignments. ObjectInputStream: - skip primitive/object reading if no primitive/objects in class - use shared StringBuilder for string reading (prevent superfluous object allocations) ObjectOutputStream: - skip primitive/object writing if no primitive/objects in class - use unsafe access to calculate UTF-length - use unsafe access in readBytes() and writeChars() methods to access String value field - removed cbuf field ObjectStreamClass/ObjectStreamField: - minor improvement in getClassSignature ; share method code with ObjectStreamField diff --git a/src/share/classes/java/io/ObjectInputStream.java b/src/share/classes/java/io/ObjectInputStream.java --- a/src/share/classes/java/io/ObjectInputStream.java +++ b/src/share/classes/java/io/ObjectInputStream.java @@ -39,8 +39,8 @@ import java.util.HashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; import static java.io.ObjectStreamClass.processQueue; + import sun.reflect.misc.ReflectUtil;
/** @@ -534,7 +534,7 @@ if (ctx == null) { throw new NotActiveException("not in call to readObject"); } - Object curObj = ctx.getObj(); + ctx.getObj(); ObjectStreamClass curDesc = ctx.getDesc(); bin.setBlockDataMode(false); GetFieldImpl getField = new GetFieldImpl(curDesc); @@ -1597,7 +1597,7 @@ int descHandle = handles.assign(unshared ? unsharedMarker : desc); passHandle = NULL_HANDLE;
- ObjectStreamClass readDesc = null; + ObjectStreamClass readDesc; try { readDesc = readClassDescriptor(); } catch (ClassNotFoundException ex) { @@ -1976,29 +1976,34 @@ }
int primDataSize = desc.getPrimDataSize(); - if (primVals == null || primVals.length < primDataSize) { - primVals = new byte[primDataSize]; - } - bin.readFully(primVals, 0, primDataSize, false); - if (obj != null) { - desc.setPrimFieldValues(obj, primVals); - } - - int objHandle = passHandle; - ObjectStreamField[] fields = desc.getFields(false); - Object[] objVals = new Object[desc.getNumObjFields()]; - int numPrimFields = fields.length - objVals.length; - for (int i = 0; i < objVals.length; i++) { - ObjectStreamField f = fields[numPrimFields + i]; - objVals[i] = readObject0(f.isUnshared()); - if (f.getField() != null) { - handles.markDependency(objHandle, passHandle); + if (primDataSize > 0) { + if (primVals == null || primVals.length < primDataSize) { + primVals = new byte[ ((primDataSize>>4)+1)<<4 ]; + } + bin.readFully(primVals, 0, primDataSize, false); + if (obj != null) { + desc.setPrimFieldValues(obj, primVals); } } - if (obj != null) { - desc.setObjFieldValues(obj, objVals); + + int numObjFields = desc.getNumObjFields(); + if (numObjFields > 0) { + int objHandle = passHandle; + ObjectStreamField[] fields = desc.getFields(false); + Object[] objVals = new Object[numObjFields]; + int numPrimFields = fields.length - objVals.length; + for (int i = 0; i < objVals.length; i++) { + ObjectStreamField f = fields[numPrimFields + i]; + objVals[i] = readObject0(f.isUnshared()); + if (f.getField() != null) { + handles.markDependency(objHandle, passHandle); + } + } + if (obj != null) { + desc.setObjFieldValues(obj, objVals); + } + passHandle = objHandle; } - passHandle = objHandle; }
/** @@ -2377,8 +2382,10 @@ private final byte[] buf = new byte[MAX_BLOCK_SIZE]; /** buffer for reading block data headers */ private final byte[] hbuf = new byte[MAX_HEADER_SIZE]; - /** char buffer for fast string reads */ + /** char buffer for fast string reads - used by {@link #readUTFSpan(long)} */ private final char[] cbuf = new char[CHAR_BUF_SIZE]; + /** shared string builder for less object allocations - used by {@link #readUTFBody(long)}, {@link #readUTFChar(long)} and {@link #readUTFSpan(long)} */ + private final StringBuilder sbuf = new StringBuilder(CHAR_BUF_SIZE);
/** block data mode */ private boolean blkmode = false; @@ -3044,19 +3051,19 @@ * utflen bytes. */ private String readUTFBody(long utflen) throws IOException { - StringBuilder sbuf = new StringBuilder(); if (!blkmode) { end = pos = 0; }
+ sbuf.setLength(0); while (utflen > 0) { int avail = end - pos; if (avail >= 3 || (long) avail == utflen) { - utflen -= readUTFSpan(sbuf, utflen); + utflen -= readUTFSpan(utflen); } else { if (blkmode) { // near block boundary, read one byte at a time - utflen -= readUTFChar(sbuf, utflen); + utflen -= readUTFChar(utflen); } else { // shift and refill buffer manually if (avail > 0) { @@ -3076,10 +3083,10 @@ * Reads span of UTF-encoded characters out of internal buffer * (starting at offset pos and ending at or before offset end), * consuming no more than utflen bytes. Appends read characters to - * sbuf. Returns the number of bytes consumed. + * {@link #sbuf}. Returns the number of bytes consumed. */ - private long readUTFSpan(StringBuilder sbuf, long utflen) - throws IOException + private long readUTFSpan(long utflen) + throws IOException { int cpos = 0; int start = pos; @@ -3111,19 +3118,19 @@ throw new UTFDataFormatException(); } cbuf[cpos++] = (char) (((b1 & 0x1F) << 6) | - ((b2 & 0x3F) << 0)); + (b2 & 0x3F)); break;
case 14: // 3 byte format: 1110xxxx 10xxxxxx 10xxxxxx b3 = buf[pos + 1]; - b2 = buf[pos + 0]; + b2 = buf[pos ]; pos += 2; if ((b2 & 0xC0) != 0x80 || (b3 & 0xC0) != 0x80) { throw new UTFDataFormatException(); } cbuf[cpos++] = (char) (((b1 & 0x0F) << 12) | - ((b2 & 0x3F) << 6) | - ((b3 & 0x3F) << 0)); + ((b2 & 0x3F) << 6) | + (b3 & 0x3F)); break;
default: // 10xx xxxx, 1111 xxxx @@ -3150,12 +3157,12 @@
/** * Reads in single UTF-encoded character one byte at a time, appends - * the character to sbuf, and returns the number of bytes consumed. + * the character to {@link #sbuf}, and returns the number of bytes consumed. * This method is used when reading in UTF strings written in block * data mode to handle UTF-encoded characters which (potentially) * straddle block-data boundaries. */ - private int readUTFChar(StringBuilder sbuf, long utflen) + private int readUTFChar(long utflen) throws IOException { int b1, b2, b3; @@ -3181,8 +3188,7 @@ if ((b2 & 0xC0) != 0x80) { throw new UTFDataFormatException(); } - sbuf.append((char) (((b1 & 0x1F) << 6) | - ((b2 & 0x3F) << 0))); + sbuf.append((char) (((b1 & 0x1F) << 6) | (b2 & 0x3F))); return 2;
case 14: // 3 byte format: 1110xxxx 10xxxxxx 10xxxxxx @@ -3198,8 +3204,8 @@ throw new UTFDataFormatException(); } sbuf.append((char) (((b1 & 0x0F) << 12) | - ((b2 & 0x3F) << 6) | - ((b3 & 0x3F) << 0))); + ((b2 & 0x3F) << 6) | + (b3 & 0x3F))); return 3;
default: // 10xx xxxx, 1111 xxxx diff --git a/src/share/classes/java/io/ObjectOutputStream.java b/src/share/classes/java/io/ObjectOutputStream.java --- a/src/share/classes/java/io/ObjectOutputStream.java +++ b/src/share/classes/java/io/ObjectOutputStream.java @@ -35,7 +35,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import static java.io.ObjectStreamClass.processQueue; -import java.io.SerialCallbackContext; + +import sun.misc.Unsafe; import sun.reflect.misc.ReflectUtil;
/** @@ -458,7 +459,7 @@ if (ctx == null) { throw new NotActiveException("not in call to writeObject"); } - Object curObj = ctx.getObj(); + ctx.getObj(); ObjectStreamClass curDesc = ctx.getDesc(); curPut = new PutFieldImpl(curDesc); } @@ -1300,7 +1301,7 @@ */ private void writeString(String str, boolean unshared) throws IOException { handles.assign(unshared ? null : str); - long utflen = bout.getUTFLength(str); + long utflen = BlockDataOutputStream.getUTFLength(str); if (utflen <= 0xFFFF) { bout.writeByte(TC_STRING); bout.writeUTF(str, utflen); @@ -1527,29 +1528,34 @@ desc.checkDefaultSerialize();
int primDataSize = desc.getPrimDataSize(); - if (primVals == null || primVals.length < primDataSize) { - primVals = new byte[primDataSize]; + if (primDataSize > 0) { + if (primVals == null || primVals.length < primDataSize) { + primVals = new byte[ ((primDataSize>>4)+1)<<4 ]; + } + desc.getPrimFieldValues(obj, primVals); + bout.write(primVals, 0, primDataSize, false); } - desc.getPrimFieldValues(obj, primVals); - bout.write(primVals, 0, primDataSize, false);
- ObjectStreamField[] fields = desc.getFields(false); - Object[] objVals = new Object[desc.getNumObjFields()]; - int numPrimFields = fields.length - objVals.length; - desc.getObjFieldValues(obj, objVals); - for (int i = 0; i < objVals.length; i++) { - if (extendedDebugInfo) { - debugInfoStack.push( - "field (class \"" + desc.getName() + "\", name: \"" + - fields[numPrimFields + i].getName() + "\", type: \"" + - fields[numPrimFields + i].getType() + "\")"); - } - try { - writeObject0(objVals[i], - fields[numPrimFields + i].isUnshared()); - } finally { + int numObjFields = desc.getNumObjFields(); + if (numObjFields > 0) { + ObjectStreamField[] fields = desc.getFields(false); + Object[] objVals = new Object[numObjFields]; + int numPrimFields = fields.length - objVals.length; + desc.getObjFieldValues(obj, objVals); + for (int i = 0; i < objVals.length; i++) { if (extendedDebugInfo) { - debugInfoStack.pop(); + debugInfoStack.push( + "field (class \"" + desc.getName() + "\", name: \"" + + fields[numPrimFields + i].getName() + "\", type: \"" + + fields[numPrimFields + i].getType() + "\")"); + } + try { + writeObject0(objVals[i], + fields[numPrimFields + i].isUnshared()); + } finally { + if (extendedDebugInfo) { + debugInfoStack.pop(); + } } } } @@ -1743,15 +1749,11 @@ private static final int MAX_BLOCK_SIZE = 1024; /** maximum data block header length */ private static final int MAX_HEADER_SIZE = 5; - /** (tunable) length of char buffer (for writing strings) */ - private static final int CHAR_BUF_SIZE = 256;
/** buffer for writing general/block data */ private final byte[] buf = new byte[MAX_BLOCK_SIZE]; /** buffer for writing block data headers */ private final byte[] hbuf = new byte[MAX_HEADER_SIZE]; - /** char buffer for fast string writes */ - private final char[] cbuf = new char[CHAR_BUF_SIZE];
/** block data mode */ private boolean blkmode = false; @@ -1763,6 +1765,18 @@ /** loopback stream (for data writes that span data blocks) */ private final DataOutputStream dout;
+ /** use unsafe to directly access value field in java.lang.String */ + private static final Unsafe unsafe = Unsafe.getUnsafe(); + /** use field offset to directly access value field in java.lang.String */ + private static final long stringValueOffset; + static { + try { + stringValueOffset = unsafe.objectFieldOffset(String.class.getDeclaredField("value")); + } catch (NoSuchFieldException e) { + throw new InternalError(e); + } + } + /** * Creates new BlockDataOutputStream on top of given underlying stream. * Block data mode is turned off by default. @@ -1972,35 +1986,23 @@ }
public void writeBytes(String s) throws IOException { - int endoff = s.length(); - int cpos = 0; - int csize = 0; + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + int endoff = sChars.length; for (int off = 0; off < endoff; ) { - if (cpos >= csize) { - cpos = 0; - csize = Math.min(endoff - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - } if (pos >= MAX_BLOCK_SIZE) { drain(); } - int n = Math.min(csize - cpos, MAX_BLOCK_SIZE - pos); + int n = Math.min(endoff - off, MAX_BLOCK_SIZE - pos); int stop = pos + n; while (pos < stop) { - buf[pos++] = (byte) cbuf[cpos++]; + buf[pos++] = (byte) sChars[off++]; } - off += n; } }
public void writeChars(String s) throws IOException { - int endoff = s.length(); - for (int off = 0; off < endoff; ) { - int csize = Math.min(endoff - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - writeChars(cbuf, 0, csize); - off += csize; - } + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + writeChars(sChars, 0, sChars.length); }
public void writeUTF(String s) throws IOException { @@ -2130,25 +2132,21 @@ }
/** - * Returns the length in bytes of the UTF encoding of the given string. + * Returns the length in bytes of the UTF encoding of this given string. */ - long getUTFLength(String s) { - int len = s.length(); + static long getUTFLength(String s) { + char[] value = (char[])unsafe.getObject(s, stringValueOffset); + int len = value.length; long utflen = 0; for (int off = 0; off < len; ) { - int csize = Math.min(len - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - for (int cpos = 0; cpos < csize; cpos++) { - char c = cbuf[cpos]; - if (c >= 0x0001 && c <= 0x007F) { - utflen++; - } else if (c > 0x07FF) { - utflen += 3; - } else { - utflen += 2; - } + char c = value[off++]; + if (c >= 0x0001 && c <= 0x007F) { + utflen++; + } else if (c > 0x07FF) { + utflen += 3; + } else { + utflen += 2; } - off += csize; } return utflen; } @@ -2198,40 +2196,36 @@ * 8-byte length header) of the UTF encoding for the given string. */ private void writeUTFBody(String s) throws IOException { + char[] sChars = (char[])unsafe.getObject(s, stringValueOffset); + int len = sChars.length; int limit = MAX_BLOCK_SIZE - 3; - int len = s.length(); for (int off = 0; off < len; ) { - int csize = Math.min(len - off, CHAR_BUF_SIZE); - s.getChars(off, off + csize, cbuf, 0); - for (int cpos = 0; cpos < csize; cpos++) { - char c = cbuf[cpos]; - if (pos <= limit) { - if (c <= 0x007F && c != 0) { - buf[pos++] = (byte) c; - } else if (c > 0x07FF) { - buf[pos + 2] = (byte) (0x80 | ((c >> 0) & 0x3F)); - buf[pos + 1] = (byte) (0x80 | ((c >> 6) & 0x3F)); - buf[pos + 0] = (byte) (0xE0 | ((c >> 12) & 0x0F)); - pos += 3; - } else { - buf[pos + 1] = (byte) (0x80 | ((c >> 0) & 0x3F)); - buf[pos + 0] = (byte) (0xC0 | ((c >> 6) & 0x1F)); - pos += 2; - } - } else { // write one byte at a time to normalize block - if (c <= 0x007F && c != 0) { - write(c); - } else if (c > 0x07FF) { - write(0xE0 | ((c >> 12) & 0x0F)); - write(0x80 | ((c >> 6) & 0x3F)); - write(0x80 | ((c >> 0) & 0x3F)); - } else { - write(0xC0 | ((c >> 6) & 0x1F)); - write(0x80 | ((c >> 0) & 0x3F)); - } + char c = sChars[off++]; + if (pos <= limit) { + if (c <= 0x007F && c != 0) { + buf[pos++] = (byte) c; + } else if (c > 0x07FF) { + buf[pos + 2] = (byte) (0x80 | ( c & 0x3F)); + buf[pos + 1] = (byte) (0x80 | ((c >> 6) & 0x3F)); + buf[pos ] = (byte) (0xE0 | ((c >> 12) & 0x0F)); + pos += 3; + } else { + buf[pos + 1] = (byte) (0x80 | ( c & 0x3F)); + buf[pos ] = (byte) (0xC0 | ((c >> 6) & 0x1F)); + pos += 2; + } + } else { // write one byte at a time to normalize block + if (c <= 0x007F && c != 0) { + write(c); + } else if (c > 0x07FF) { + write(0xE0 | ((c >> 12) & 0x0F)); + write(0x80 | ((c >> 6) & 0x3F)); + write(0x80 | ( c & 0x3F)); + } else { + write(0xC0 | ((c >> 6) & 0x1F)); + write(0x80 | ( c & 0x3F)); } } - off += csize; } } } @@ -2464,7 +2458,10 @@ StringBuilder buffer = new StringBuilder(); if (!stack.isEmpty()) { for(int i = stack.size(); i > 0; i-- ) { - buffer.append(stack.get(i-1) + ((i != 1) ? "\n" : "")); + buffer.append(stack.get(i - 1)); + if (i!=1) { + buffer.append('\n'); + } } } return buffer.toString(); diff --git a/src/share/classes/java/io/ObjectStreamClass.java b/src/share/classes/java/io/ObjectStreamClass.java --- a/src/share/classes/java/io/ObjectStreamClass.java +++ b/src/share/classes/java/io/ObjectStreamClass.java @@ -1474,7 +1474,28 @@ /** * Returns JVM type signature for given class. */ - private static String getClassSignature(Class<?> cl) { + static String getClassSignature(Class<?> cl) { + if (cl.isPrimitive()) + if (cl == Integer.TYPE) { + return "I"; + } else if (cl == Byte.TYPE) { + return "B"; + } else if (cl == Long.TYPE) { + return "J"; + } else if (cl == Float.TYPE) { + return "F"; + } else if (cl == Double.TYPE) { + return "D"; + } else if (cl == Short.TYPE) { + return "S"; + } else if (cl == Character.TYPE) { + return "C"; + } else if (cl == Boolean.TYPE) { + return "Z"; + } else if (cl == Void.TYPE) { + return "V"; + } + StringBuilder sbuf = new StringBuilder(); while (cl.isArray()) { sbuf.append('['); @@ -1503,7 +1524,7 @@ throw new InternalError(); } } else { - sbuf.append('L' + cl.getName().replace('.', '/') + ';'); + sbuf.append('L').append(cl.getName().replace('.', '/')).append(';'); } return sbuf.toString(); } diff --git a/src/share/classes/java/io/ObjectStreamField.java b/src/share/classes/java/io/ObjectStreamField.java --- a/src/share/classes/java/io/ObjectStreamField.java +++ b/src/share/classes/java/io/ObjectStreamField.java @@ -91,7 +91,7 @@ this.name = name; this.type = type; this.unshared = unshared; - signature = getClassSignature(type).intern(); + signature = ObjectStreamClass.getClassSignature(type).intern(); field = null; }
@@ -137,7 +137,7 @@ name = field.getName(); Class<?> ftype = field.getType(); type = (showType || ftype.isPrimitive()) ? ftype : Object.class; - signature = getClassSignature(ftype).intern(); + signature = ObjectStreamClass.getClassSignature(ftype).intern(); }
/** @@ -286,41 +286,4 @@ String getSignature() { return signature; } - - /** - * Returns JVM type signature for given class. - */ - private static String getClassSignature(Class<?> cl) { - StringBuilder sbuf = new StringBuilder(); - while (cl.isArray()) { - sbuf.append('['); - cl = cl.getComponentType(); - } - if (cl.isPrimitive()) { - if (cl == Integer.TYPE) { - sbuf.append('I'); - } else if (cl == Byte.TYPE) { - sbuf.append('B'); - } else if (cl == Long.TYPE) { - sbuf.append('J'); - } else if (cl == Float.TYPE) { - sbuf.append('F'); - } else if (cl == Double.TYPE) { - sbuf.append('D'); - } else if (cl == Short.TYPE) { - sbuf.append('S'); - } else if (cl == Character.TYPE) { - sbuf.append('C'); - } else if (cl == Boolean.TYPE) { - sbuf.append('Z'); - } else if (cl == Void.TYPE) { - sbuf.append('V'); - } else { - throw new InternalError(); - } - } else { - sbuf.append('L' + cl.getName().replace('.', '/') + ';'); - } - return sbuf.toString(); - } } *Gesendet:* Dienstag, 07. Januar 2014 um 10:05 Uhr *Von:* "Chris Hegarty" <chris.hegarty@oracle.com> *An:* "Robert Stupp" <snazy@gmx.de> *Cc:* "core-libs-dev@openjdk.java.net Libs" < core-libs-dev@openjdk.java.net> *Betreff:* Re: ObjectIn/OutputStream improvements On 15 Dec 2013, at 10:29, Robert Stupp <snazy@gmx.de> wrote:
Hi,
I digged through the object serialization code and found some lines that could be optimized to reduce the number of calls to System.arraycopy() and temporary object allocations especially during string (de)serialization. In short sentences the changes are: ObjectInputStream: - skip primitive/object reading if no primitive/objects in class (defaultReadFields method) - use shared StringBuilder for string reading (prevent superfluous object allocations of one StingBuilder and one implicit char[] for each string being deserialized) ObjectOutputStream: - skip primitive/object writing if no primitive/objects in class (defaultWriteFields method) - use unsafe access to calculate UTF-length - use unsafe access in readBytes() and writeChars() methods to access String value field - removed cbuf field ObjectStreamClass/ObjectStreamField: - minor improvement in getClassSignature ; share method code with ObjectStreamField (return constant string for primitives)
I have tested the changes in a big Java installation at a customer (backported the Java8 ObjectIn/OutputStream including the changes to Java6) and a long running real application performance test resulted in reduced CPU usage (from about 60% per server to 50%). The changes I made in openjdk8 pass all tests.
Since I have no experience how to contribute code to openjdk in form of a push/changeset I have added the diff (hg export -g) to this email.
Did you attach the diffs? I don’t see any attachment, it may be that the attachment was stripped. Can you try resending inline?
You should take a look at the OpenJDK How to Contribute page [1]. Paying particular attention to the OCA, without it we will not be able to move your patch forward.
Thanks, -Chris.
[1] http://openjdk.java.net/contribute/
Robert
participants (9)
-
Chris Hegarty
-
Martin Buchholz
-
Mike Duigou
-
Paul Sandoz
-
Peter Levart
-
Robert Stupp
-
Stuart Marks
-
Vitaly Davidovich
-
Xueming Shen