On Apr 2 2013, at 04:07 , Alan Bateman wrote: > On 02/04/2013 05:44, Mike Duigou wrote: >> Hello all; >> >> Last night while pushing another changeset I accidentally pushed a changeset >> for JKD-7143928. Since the review and testing was not complete on this issue >> I have backed out that changeset and created a new bug number to continue >> development. The new webrev to complete the review is: >> >> http://cr.openjdk.java.net/~mduigou/JDK-8011200/0/webrev/ >> >> It is currently unchanged from the last posted changeset for 7143928. >> >> Mike >> > I skimmed through the webrev as lazily creating the backing arrays will be > help in some environments. > > For ArrayList.readObject then you read the array length in as > "initialCapacity" which I think is a bit confusing given the current > semantics. An alternative is to just not change readObject unless there is > evidence that it is common to reconstitute empty ArrayLists.
I've gone with making ArrayList.writeObject/readObject behave like clone(). They remain forward and backwards compatible with existing impls. > For HashMap.indexFor then I assume this isn't an assert because it is used > early in the VM startup. Correct. > This should probably be changed to InternalError and the message needs to be > changed too. It will be removed. :-) I had put this in for my testing. > > Do you envisage usage of inflateTable in LinkedHashMap? I'm wondering why it > isn't private. I wasn't sure when I wrote it. I will make it private. > HashMap.writeObject - the update to the @serialData text will change the > wording emitting in the javadoc. The "must be a power of 2" is actually an existing but previously undocumented requirement. I will file a CCC case for this. > I mention this as I think you are planning to push this to jdk7u at some > point. Even though it's been a requirement for the life of this class I won't attempt to push this change to jdk7u. > I don't have time to go through the test in detail at this time but it looks > like it has the original bug number and I assume that will need to change now. Correct. It will be changed. Transitioning it now. Mike
