chris.hegarty at oracle.com
Fri Jan 31 19:26:36 UTC 2014
On 31 Jan 2014, at 19:07, Stuart Marks <stuart.marks at 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().
More information about the core-libs-dev