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
