Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-27 Thread Paul Sandoz
On Feb 26, 2015, at 12:38 PM, Chris Hegarty chris.hega...@oracle.com wrote: On 24 Feb 2015, at 15:07, Chris Hegarty chris.hega...@oracle.com wrote: On 24 Feb 2015, at 11:45, Peter Levart peter.lev...@gmail.com wrote: ... That's better now. Let me just try to measure the overhead of

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-27 Thread Peter Levart
Hi Chris, webrev.04 is nice an simple now. I'm ok with it. Regards, Peter On Feb 27, 2015 11:11 AM, Chris Hegarty chris.hega...@oracle.com wrote: Hi Peter, I think you are ok with this now. If so, would you mind to just taking a quick look at the webrev, the changes are trivial, and

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-26 Thread Chris Hegarty
On 24 Feb 2015, at 15:07, Chris Hegarty chris.hega...@oracle.com wrote: On 24 Feb 2015, at 11:45, Peter Levart peter.lev...@gmail.com wrote: ... That's better now. Let me just try to measure the overhead of tracking on simple objects to see if it actually pays off or is contra-productive in

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-26 Thread Chris Hegarty
On 26 Feb 2015, at 11:38, Chris Hegarty chris.hega...@oracle.com wrote: On 24 Feb 2015, at 15:07, Chris Hegarty chris.hega...@oracle.com wrote: On 24 Feb 2015, at 11:45, Peter Levart peter.lev...@gmail.com wrote: ... That's better now. Let me just try to measure the overhead of tracking on

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-24 Thread Paul Sandoz
On Feb 23, 2015, at 10:08 PM, Peter Levart peter.lev...@gmail.com wrote: - We have to be careful with loosening of volatile writes to final fields in custom readObject() methods (BigDecimal.intCompact for example) especialy if they are writes to fields that are not serial fields in

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-24 Thread Peter Levart
Hi Chris, On 02/24/2015 11:53 AM, Chris Hegarty wrote: Peter, On 23 Feb 2015, at 21:08, Peter Levart peter.lev...@gmail.com wrote: Hi Chris, On 02/23/2015 12:01 PM, Chris Hegarty wrote: Peter, David, Vitaly, Can you please take a look at the latest version of this change:

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-24 Thread Chris Hegarty
On 24 Feb 2015, at 11:45, Peter Levart peter.lev...@gmail.com wrote: ... - You are tracking the requiresFreeze flag in readSerialData() method for each class slot the deserialized object is composed of. This can be optimized and the 'hasFinalField' flag pre-computed for the whole object

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Chris Hegarty
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/ 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

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Vitaly Davidovich
Likewise, seems fine. 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? Also, hasFinal field can be final, unless I missed some context in the webrev. sent from my phone On Feb 23, 2015 7:30 AM, David

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread David Holmes
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/ It seems reasonable but I don't have a clear picture of the connection between

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Chris Hegarty
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

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Vitaly Davidovich
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.

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Chris Hegarty
On 23/02/15 12:30, David Holmes 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/ It seems reasonable Thanks. but I

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Chris Hegarty
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

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Paul Sandoz
On Feb 23, 2015, at 4:40 PM, Chris Hegarty chris.hega...@oracle.com wrote: 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

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Chris Hegarty
On 23/02/15 15:59, Paul Sandoz wrote: On Feb 23, 2015, at 4:40 PM, Chris Hegarty chris.hega...@oracle.com wrote: 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

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-23 Thread Peter Levart
Hi Chris, On 02/23/2015 12: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/ There are still a couple of issues with this version: - You are issuing freeze

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-20 Thread Chris Hegarty
On 20/02/15 00:27, David Holmes wrote: On 20/02/2015 10:13 AM, Vitaly Davidovich wrote: In addition to Peter's comment, full fence seems unnecessarily strong and will cause performance issues (especially if the fence is per object in the graph). A storeFence should be sufficient here, no? It

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-20 Thread Vitaly Davidovich
Chris, I think it's fine if there are intra graph object references during deserialization - nothing unexpected should happen since intra thread semantics will be preserved. The freeze action is only a concern once deserialization completes and user code may publish an object from the graph

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-20 Thread Chris Hegarty
On 20/02/15 14:25, Vitaly Davidovich wrote: Chris, I think it's fine if there are intra graph object references during deserialization - nothing unexpected should happen since intra thread semantics will be preserved. The freeze action is only a concern once deserialization completes and user

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-20 Thread Peter Levart
On 02/20/2015 12:34 PM, Chris Hegarty wrote: Updated Webrev: http://cr.openjdk.java.net/~chegar/deserialFence/webrev.01/webrev/ Note, the changes in this webrev are overly defensive in the face of recursive calls to readObject/Unshared. This should be ok, but probably not strictly

RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-19 Thread Chris Hegarty
A number of years ago there was a proposal to use Unsafe.put*Volatile methods to set final fields during default deserialisation [1][2], but it never made it due to concerns about the potential negative impact of the additional fences. Now we have a, private, fences API in the platform, I think

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-19 Thread Peter Levart
Hi Chris, Would it be possible to track the needsFence flag throughout the whole object graph deserialization, and only issue one single fence for the whole object graph? This would minimize the number of fences issued. Peter On 02/19/2015 05:32 PM, Chris Hegarty wrote: Additional note (

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-19 Thread David Holmes
On 20/02/2015 10:13 AM, Vitaly Davidovich wrote: In addition to Peter's comment, full fence seems unnecessarily strong and will cause performance issues (especially if the fence is per object in the graph). A storeFence should be sufficient here, no? It should be a fence per graph, or perhaps

Re: RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields

2015-02-19 Thread Vitaly Davidovich
In addition to Peter's comment, full fence seems unnecessarily strong and will cause performance issues (especially if the fence is per object in the graph). A storeFence should be sufficient here, no? sent from my phone On Feb 19, 2015 11:32 AM, Chris Hegarty chris.hega...@oracle.com wrote: