atom feed19 messages in edu.oswego.cs.concurrency-interestRe: [concurrency-interest] The Atomic...
FromSent OnAttachments
David M. LloydJul 14, 2012 11:01 am 
Doug LeaJul 14, 2012 11:17 am 
Stanimir SimeonoffJul 14, 2012 1:01 pm 
√iktor ҠlangJul 14, 2012 1:16 pm 
Rémi ForaxJul 14, 2012 2:20 pm 
Stanimir SimeonoffJul 14, 2012 2:58 pm 
Rémi ForaxJul 14, 2012 3:44 pm 
Doug LeaJul 15, 2012 3:37 am 
David HolmesJul 15, 2012 4:14 am 
√iktor ҠlangJul 15, 2012 8:22 am 
David M. LloydJul 15, 2012 9:19 am 
Jed Wesley-SmithJul 15, 2012 7:31 pm 
David HolmesJul 15, 2012 7:35 pm 
David M. LloydJul 15, 2012 7:43 pm 
David HolmesJul 15, 2012 7:52 pm 
Jason T. GreeneJul 20, 2012 11:42 am 
David HolmesJul 20, 2012 6:34 pm 
David M. LloydJul 22, 2012 1:47 pm 
Doug LeaJul 23, 2012 3:44 am 
Subject:Re: [concurrency-interest] The Atomic*FieldUpdater situation
From:David Holmes (davi@aapt.net.au)
Date:Jul 15, 2012 7:35:41 pm
List:edu.oswego.cs.concurrency-interest

David,

David M. Lloyd writes:

On 07/15/2012 06:14 AM, David Holmes wrote:

David M. Lloyd writes:

What is the purpose of the access-time access check in the atomic field updater classes?

Do you mean the ensureProtectedAccess check?

This is to ensure that you can't get an updater for a protected inherited field in SubtypeA, then use that updater to access/modify the field in an instance of SubtypeB.

Which is ridiculous because if a subclass "inherited" access to a normal protected field on my class, I'd still be able to access it directly in the base class... because fields aren't actually inherited.

I think you are missing the point. Consider this:

class Base { protected volatile int x; }

final class Secure extends Base { ... }

Instances of Secure are only supposed to be used through their public API, as defined by Base and Secure. I hope you will agree that if I have an instance of Secure I shouldn't be able to arbitrarily mess with the value of x?

But given

class BackDoor extends Base { static AtomicIntegerFieldUpdater<Base> u = ... (Base.class, "x");

public static void setBaseX(Base target, int newX) { u.set(target, newX); } }

without these protected access checks, BackDoor.setBaseX would allow you to update _any_ instance of Base.

If I construct an atomic field updater, to me, this should be exactly equivalent to having an accessible=true Field instance. If I want to manage the visibility of that instance, I will restrict access to the updater itself.

The problem, as I am sure you are all well aware, is that when the updater classes are used on an object instance which is subclassed, even if the field being updated, the field updater, and the method calling into the field updater all exist on the base class and are private, an expensive access check has to occur.

Now you've lost me. Which access check is this? All of the methods do:

if (obj == null || obj.getClass() != tclass || cclass != null) fullCheck(obj); <snip>

So if everything is in the same class we don't do any additional checks.

I believe this is inaccurate. If I have a private static updater for a private field in the base class, and even if I access the updater only from the base class, the additional checks are still run because obj.getClass() != tclass (it's actually a subclass, not tclass).

Sorry. I stated "if everything is in the same class" and here the target object is not the same class. I misinterpreted the conditions you were establishing.

Access checks are always pure overhead for correct code. But they aren't there for correct code.

David

But even if it weren't, the original access check is relatively expensive and completely unneeded. Even assuming these access checks aren't completely pointless, which of course they are - if I want public access to a private field, I'd make the updater public. If I want private access then the updater will be private. No access-time checks are necessary; all the appropriate verification should have been done (usually at class init) when the updater is constructed by examining the caller class. And I strongly, *strongly* doubt there is any code in the wild which relies on these access-time checks to ensure access protection.

And while I'm on this tirade - the whole design of using subclasses for the different variations in updater implementation seems unnecessary. If it were all in one final class, and the VM_SUPPORTS_LONG_CAS flag were a constant static field (which of course it is), then having e.g. compareAndSet be implemented like this:

public /*final*/ boolean compareAndSet(T obj, long expect, long update) { if (VM_SUPPORTS_LONG_CAS) { return unsafe.compareAndSwapLong(obj, offset, expect, update); } else synchronized (this) { long v = unsafe.getLong(obj, offset); if (v != expect) return false; unsafe.putLong(obj, offset, update); return true; } }

...it seems to me that the method is much more likely to be inlined, and furthermore the un-followed branch should be elided, which would essentially make this a single instruction on most platforms.

Even when the class is final, the access check is still expensive relative to the operation itself!

I mean I'm using field updaters in the first place because I can't afford to have a jillion Atomic* objects floating around; obviously performance is critical here. And to add to this problem, you can't even rely on using Unsafe, even though it's becoming more and more prevalent in JDKs, as some platforms may not have atomic 64-bit operations, and there's no way I can see to test for that other than relying on the AtomicLong implementation to be similar to OpenJDK's.

Is there any recourse to solve this issue?