Hi Stuart On Feb 28, 2014, at 5:35 PM, Stuart Marks wrote:
> This is pretty good. After this long, strange trip through the JMM, restoring > the sentinel values to zeroes and renaming the fields to be explicit about > how they represent the actual values seems to be the best approach. Paul's > suggestion about using the term "stable value" in comments is good too. > > I took a look at the serialization stuff. The actual serialized form hasn't > changed, so there should be no compatibility here with previous versions. For the above, good! > There are some things in the serialization doc that ought to be brought up to > date, though. Note that the docs for serialPersistentFields, readObject, and > writeObject appear in the javadoc output, in the "Serialized Form" page, even > though these members are private! > > Per another of Paul's comments, the @serial tag should be removed from > bitCountPlusOne, bitLengthPlusOne, and lowestSetBitPlusTwo, since these > fields do not appear in the serialized representation. Yes, I was wondering about that. > The fields bitCount, bitLength, and lowestSetBit appear in the serialized > form only for backward compatibility and are otherwise ignored, so their > @serialField entries should just say that instead of describing how they were > formerly used. Also, firstNonzeroByteNum is missing a @serialField entry, and > it should have the same description as the others. > > Typo at 4236-4237, it says "be\ndefault" instead of "by\ndefault". > > The comment at lines 4242-4246 should simply be removed. The first and third > sentences are redundant with other docs. The second sentence, "The magnitude > field is used as a temporary store for the byte array that is deserialized" > is incorrect, as there is no longer a 'magnitude' field; a local is used > instead. > > The @serialData tag at line 4316 for writeObject is misused; this is really > intended for *extra* serial data written by writeObject after the > writeFields() or defaultWriteObject() call, which doesn't occur here. It > might be worth being explicit in writeObject's doc comment about writing -1's > and -2's as the values for bitCount, bitLength, lowestSetBit, and > firstNonzeroByteNum for compatibility with older implementations, even though > current implementations will ignore these values. I will make the suggested updates and repost an updated webrev, probably on Monday. Thanks for the detailed review! Brian
