RFR: 8256480: Refactor ObjectInputStream field reader implementation

[Roger Riggs]

Hi Brent,

Thanks for the review

On 11/18/20 7:24 PM, Brent Christian wrote:
> Hi, Roger.
>
> The change looks good.  I just noticed a couple small things:
>
> 2324             for (int i = 0; i < slots.length-1; i++) {
> 2325                 new FieldValues(slots[i].desc, true);
>
> The slots[i].hasData check is no longer performed here.

I'll restore that optimization; Without the optimization a FieldValues 
object would be created
with neither primitive or object values only to be discarded.
>
> 2561             primValues = new byte[desc.getPrimDataSize()];
> 2562             objValues = new Object[desc.getNumObjFields()];
>
> In defaultReadFields(), the arrays would remain null in the case of a 
> 0 from getPrimDataSize() or getNumObjFields().  In the new code, 
> 0-length arrays are created.
True, the implementations differ on this point.
Retaining the zero length arrays might be slightly safer (no null checks 
required)
but there should be no references to the arrays in either case and 
allocating new arrays
is a bit of a waste.

I'll restore the unused = null datapaths; and re-test.
It will also slightly streamline the case where are there are no 
primitives or no objects.

Thanks, Roger

>
> Thanks,
> -Brent
>
> On 11/18/20 9:39 AM, Roger Riggs wrote:
>> ObjectInputStream has nearly identical but separate implementations 
>> to read values from the stream.
>> Both implementations read primitive and object values from the stream 
>> and return an object holding the values.
>> OIS.readFields() uses the internal class GetFieldImpl while 
>> OIS.defaultReadObject and reading records uses the internal class 
>> FieldValues.
>> The behavioral difference between the two is whether dependencies are 
>> tracked in the object handle table or not.
>>
>> The classes are merged, retaining the internal FieldValues name and 
>> the behavior to track dependencies or not.
>> The constructor is passed the class descriptor and flag to track or 
>> not; it reads and saves the values from the stream.
>> The callers are updated to call the merge FieldValues methods.
>>
>> There is no change in behavior; all current tests pass.
>>
>> -------------
>>
>> Commit messages:
>>   - 8256480: Refactor ObjectInputStream field reader implementation
>>
>> Changes: https://git.openjdk.java.net/jdk/pull/1296/files
>>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1296&range=00
>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8256480
>>    Stats: 157 lines in 1 file changed: 26 ins; 78 del; 53 mod
>>    Patch: https://git.openjdk.java.net/jdk/pull/1296.diff
>>    Fetch: git fetch https://git.openjdk.java.net/jdk 
>> pull/1296/head:pull/1296
>>
>> PR: https://git.openjdk.java.net/jdk/pull/1296
>>