On 23/02/15 15:30, Vitaly Davidovich wrote:
Ok Chris, sounds good.
It could be, but I omitted it as it requires a pesky explicit
assignment of false in the case where there are not final fields!
You could use a local (non-final) boolean to track this state (assigned
with false), and then set the field with it after the looping is over.
Anyway, your call.
That is better. Done.
Thanks,
-Chris.
On Mon, Feb 23, 2015 at 10:32 AM, Chris Hegarty
<chris.hega...@oracle.com <mailto:chris.hega...@oracle.com>> wrote:
On 23/02/15 14:08, Vitaly Davidovich wrote:
Likewise, seems fine.
Thanks Vitaly.
By the way, is there a reason not to call
freeze() right before returning obj? Is there something special
about
the place it's invoked at now?
Probably not. The freeze should probably happen before the
ObjectInputValidation callback, as this justs opens another
opportunity for early publication of the object, but probably after
the handle update and check.
Also, hasFinal field can be final, unless I missed some context
in the
webrev.
It could be, but I omitted it as it requires a pesky explicit
assignment of false in the case where there are not final fields!
For completeness, updated webrev in-place, which I intend to push,
unless there are further comments:
http://cr.openjdk.java.net/~__chegar/deserialFence/webrev.__02/webrev/
<http://cr.openjdk.java.net/~chegar/deserialFence/webrev.02/webrev/>
-Chris.
sent from my phone
On Feb 23, 2015 7:30 AM, "David Holmes" <david.hol...@oracle.com
<mailto:david.hol...@oracle.com>
<mailto:david.holmes@oracle.__com
<mailto:david.hol...@oracle.com>>> wrote:
Hi Chris,
On 23/02/2015 9:01 PM, Chris Hegarty wrote:
Peter, David, Vitaly,
Can you please take a look at the latest version of
this change:
http://cr.openjdk.java.net/~____chegar/deserialFence/webrev.____02/webrev/
<http://cr.openjdk.java.net/~__chegar/deserialFence/webrev.__02/webrev/>
<http://cr.openjdk.java.net/~__chegar/deserialFence/webrev.__02/webrev/
<http://cr.openjdk.java.net/~chegar/deserialFence/webrev.02/webrev/>>
It seems reasonable but I don't have a clear picture of the
connection between readObject and readSerialData.
David
On 20/02/15 15:09, Peter Levart wrote:
...
This looks good now. But I wonder if issuing fences
after
nested calls
to readObject() makes much sense. If cycles are
present in a
subgraph
deserialized by nested call to readObject(), a field
pointing back to an
object not part of the subgraph stream will point
to the
object that
will not be fully initialized yet, so nested calls to
readObject()
should not be expected to return a reference to a fully
constructed
subgraph anyway. Only top-level call is guaranteed
to return
a fully
constructed graph.
Right. I was never convinced of this myself either.
Removed.
Unnecessary
complication.
If you optimize this and only issue one fence for
top-level
call to
readObject(), tracking of 'requiresFence' (I would
call it
requiresFreeze to be in line with JMM terminology - the
fence is just a
'requiresFreeze' is better. Updated
way to achieve freeze) can also be micro-optimized. You
currently do it
like this:
1900 requiresFence |=
slotDesc.hasFinalField();
which is a shortcut for:
requiresFence = requiresFence |
slotDesc.hasFinalField();
...which means that the field is overwritten
multiple times
unnecessarily and slotDesc.hasFinalField() is
called every
time. You can
write the same logic as:
if (!requiresFence && slotDesc.hasFinalField()) {
requiresFence = true;
}
... and it is more readable. Updated.
There will be at most one write to the field and
potentially
less calls
to slotDesc.hasFinalField().
-Chris.