Re: RFR 8160439: Replace asserts in VarHandle.AccessMode with tests

[Paul Sandoz]

> On 28 Jun 2016, at 21:37, Martin Buchholz  wrote:
> 
> Looks good.
> 

Thanks.


> I'm still nervous about using assert in performance critical code due to 
> hotspot bytecode count inlining heuristics.
> 

The assert i left in place (because it is asserting on internal data) and the 
one what was removed were not on the critical performance path. Stil, I learned 
a lesson to be careful using asserts with code that is used early in the 
startup process.

Paul.

Re: RFR 8160439: Replace asserts in VarHandle.AccessMode with tests

[Martin Buchholz]

Looks good.

I'm still nervous about using assert in performance critical code due to
hotspot bytecode count inlining heuristics.

On Tue, Jun 28, 2016 at 3:50 AM, Paul Sandoz  wrote:

> Hi,
>
> Please review:
>
>
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8160439-vh-access-mode-remove-assert/webrev/
> <
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8160439-vh-access-mode-remove-assert/webrev/
> >
>
> This removes an assert that can load dependent locale-based classes too
> early in the startup process when say AtomicInteger is modified to use
> VarHandle (in a 166 webrev Martin has prepared). This appears to kibosh
> locale-behaviour which was thankfully caught in a test
> (java/util/Locale/Bug4152725.java).
>
> The removed assert has been replaced with a test.
>
> —
>
> Separately there might be a more principled way to ensure VarHandle and
> dependent classes are loaded/initialized at a well defined point. Although
> in this case it would not have helped, it would help define a clearer
> barrier before which a VarHandle cannot be used.
>
> Paul.
>

Re: RFR 8160439: Replace asserts in VarHandle.AccessMode with tests

[Vladimir Ivanov]

Looks good.

Best regards,
Vladimir Ivanov

On 6/28/16 1:50 PM, Paul Sandoz wrote:

Hi,

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8160439-vh-access-mode-remove-assert/webrev/
 


This removes an assert that can load dependent locale-based classes too early 
in the startup process when say AtomicInteger is modified to use VarHandle (in 
a 166 webrev Martin has prepared). This appears to kibosh locale-behaviour 
which was thankfully caught in a test (java/util/Locale/Bug4152725.java).

The removed assert has been replaced with a test.

—

Separately there might be a more principled way to ensure VarHandle and 
dependent classes are loaded/initialized at a well defined point. Although in 
this case it would not have helped, it would help define a clearer barrier 
before which a VarHandle cannot be used.

Paul.

Re: RFR 8160439: Replace asserts in VarHandle.AccessMode with tests

[Roger Riggs]

Hi Paul,

Looks fine, very straightforward

Regards, Roger


On 6/28/2016 6:50 AM, Paul Sandoz wrote:

Hi,

Please review:

   
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8160439-vh-access-mode-remove-assert/webrev/
 


This removes an assert that can load dependent locale-based classes too early 
in the startup process when say AtomicInteger is modified to use VarHandle (in 
a 166 webrev Martin has prepared). This appears to kibosh locale-behaviour 
which was thankfully caught in a test (java/util/Locale/Bug4152725.java).

The removed assert has been replaced with a test.

—

Separately there might be a more principled way to ensure VarHandle and 
dependent classes are loaded/initialized at a well defined point. Although in 
this case it would not have helped, it would help define a clearer barrier 
before which a VarHandle cannot be used.

Paul.

RFR 8160439: Replace asserts in VarHandle.AccessMode with tests

[Paul Sandoz]

Hi,

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8160439-vh-access-mode-remove-assert/webrev/
 


This removes an assert that can load dependent locale-based classes too early 
in the startup process when say AtomicInteger is modified to use VarHandle (in 
a 166 webrev Martin has prepared). This appears to kibosh locale-behaviour 
which was thankfully caught in a test (java/util/Locale/Bug4152725.java).

The removed assert has been replaced with a test.

—

Separately there might be a more principled way to ensure VarHandle and 
dependent classes are loaded/initialized at a well defined point. Although in 
this case it would not have helped, it would help define a clearer barrier 
before which a VarHandle cannot be used.

Paul.