Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure. (If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.) — John
Hi, thanks everyone for the feedback! @Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either. @John: I added the code that you suggested to simulate CAS. Please find the new webrev at: http://cr.openjdk.java.net/~anoll/8005873/webrev.02/ Sorry for the delay, I was on vacation. Thanks, Tobias On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.)
— John
Looks good. It should be safe to sync on MTF instance since it's not accessible outside (MTF and MT.form() are package-private). Best regards, Vladimir Ivanov On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks, Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.)
— John
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible outside (MTF and MT.form() are package-private).
Best regards, Vladimir Ivanov
Thank you, Vladimir. Could someone please push the patch? Thanks, Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks, Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.)
— John
Tobias, I'll take care of it. Best regards, Vladimir Ivanov On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible outside (MTF and MT.form() are package-private).
Best regards, Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks, Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks, Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.)
— John
On Jun 2, 2014, at 6:29 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
Tobias, I'll take care of it.
Are you also taking care of the backports?
Best regards, Vladimir Ivanov
On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible outside (MTF and MT.form() are package-private).
Best regards, Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks, Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks, Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.)
— John
On 6/2/14 9:23 PM, Christian Thalinger wrote:
On Jun 2, 2014, at 6:29 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
Tobias, I'll take care of it.
Are you also taking care of the backports?
Yes. I'll backport into 8u myself and for 7 I'll ask help from Sustaining. Best regards, Vladimir Ivanov
Best regards, Vladimir Ivanov
On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible outside (MTF and MT.form() are package-private).
Best regards, Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks, Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks, Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann <tobias.hartmann@oracle.com> wrote:
> Is it sufficient then to use synchronized (lambdaForms) { ... } in > setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? Yes, that is how I see it. The fast path is a racy non-volatile read of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be broken. But the JMM is carefully designed to support safe publication of array elements, and through array elements.)
— John
Hi Vladimir, On 02.06.2014 19:38, Vladimir Ivanov wrote:
On 6/2/14 9:23 PM, Christian Thalinger wrote:
On Jun 2, 2014, at 6:29 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
Tobias, I'll take care of it.
Are you also taking care of the backports?
Yes. I'll backport into 8u myself and for 7 I'll ask help from Sustaining.
Thank you! Best, Tobias
Best regards, Vladimir Ivanov
Best regards, Vladimir Ivanov
On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible outside (MTF and MT.form() are package-private).
Best regards, Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks, Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal read sees an outdated null value, a new LambdaForm is created and setCachedLambdaForm(...) is executed. This will guarantee that the non-null value is seen and used. The unnecessary creation of a new LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks, Tobias
On 19.05.2014 20:31, John Rose wrote: > On May 16, 2014, at 4:56 AM, Tobias Hartmann > <tobias.hartmann@oracle.com> wrote: > >> Is it sufficient then to use synchronized (lambdaForms) { ... } in >> setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)? > Yes, that is how I see it. The fast path is a racy non-volatile > read > of a safely-published structure. > > (If safe publication via arrays were broken, java.lang.String > would be > broken. But the JMM is carefully designed to support safe > publication > of array elements, and through array elements.) > > — John
participants (4)
-
Christian Thalinger
-
John Rose
-
Tobias Hartmann
-
Vladimir Ivanov