On 02/04/2013 23:50, Mike Duigou wrote:
Hello again;
I have updated the patch with the received review feedback. The revised webrev
is here:
http://cr.openjdk.java.net/~mduigou/JDK-8011200/1/webrev/
The important changes in this revision:
- The behaviour of the readObject/writeObject serialization for both classes now more
closely mirrors the behaviour of clone(). For ArrayList this means that the deserialized
list has a capacity the same as the size. ie. as if trimToSize() was called. For HashMap,
the opposite is true, the capacity is the same as was in effect when the object was
serialized. (HashMap also tries to protect itself from nonsensical/harmful input). The
implementation changes to serialization preserve forward and backward compatibility--all
serialized objects are compatible with all implementations. I will file a spec change
request for the addition of ", a power of 2" to the @serialData tag for this
existing but previously unstated requirement.
- Use of Arrays.fill has been reverted. I did change one fill case so that the
loop can be optimized. (size field was being updated with each iteration). I
very slightly expanded the docs.
This is starting to look like a nice set of changes.
Mike
This does looks much nicer.
For ArrayList.ensureCapacity then it might be useful to include a
comment to make it clear that it's a no-op when the ArrayList is created
with the default and ensureCapacity(n), for n < 10. I had to read it a
few times to satisfy myself that it's right.
One thing that I'm not sure about is HashMap.readObject using the bucket
count from the serialized stream. We can't predict all usages of course
but this has the potential for the table to be much bigger than we need.
Otherwise I think this looks okay to me. I didn't quite get the reason
for splitting out the update to the @serialData description but maybe
you want to separate the changes to make it easy to backport.
-Alan.
