On 31 Jan 2014, at 19:07, Stuart Marks <stuart.ma...@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/ > > 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 >
