On Thu, May 15, 2008 at 12:40 AM, 이희승 (Trustin Lee) <[EMAIL PROTECTED]> wrote:
> Yes. I was actually talking about the configuration properties which meets
> the two criteria.
>
> Of course, counters, which were originally mentioned by Dmirty, should use
> AtomicIntegers.
>
> So.. basically we were mixing up two kinds of properties in our discussion.
> To sum up:
>
> * Use volatile for simple configuration properties
> * Use AtomicInteger for other counters (except for performance counters
> which doesn't need to be accurate)
>
> Does this make sense to you guys?
Yes, I think that makes sense. I just think that in general, we have
to be very careful when using volatile instead of locking.
For the reasons explained by Brian Goetz in the article mentioned
below, and also because it might be premature optimization
(or even no optimization at all).
When I see a method like below, I wonder if that should be an atomic operation ?
It's probably not really necessary, I don't know the code well enough to judge.
public void increaseReadBytes(int increment) {
if (increment > 0) {
readBytes += increment;
lastReadTime = System.currentTimeMillis();
idleCountForBoth = 0;
idleCountForRead = 0;
}
}
Maarten
>
>
>
> On Thu, 15 May 2008 03:08:58 +0900, Maarten Bosteels
> <[EMAIL PROTECTED]> wrote:
>
>
> > Hello,
> >
> > Brian Goetz [1] writes:
> >
> > "You can use volatile variables instead of locks only under a
> > restricted set of circumstances. Both of the following criteria must
> > be met for volatile variables to provide the desired thread-safety:
> >
> > * Writes to the variable do not depend on its current value.
> > * The variable does not participate in invariants with other variables.
> > ...
> > However, if you can arrange that the value is only ever written from a
> > single thread, then you can ignore the first condition.)"
> >
> > But at line 445 of BaseIoSession.java I see:
> >
> > idleCountForRead++;
> >
> > (in a public method called increaseIdleCount so I don't think we can
> > guarantee that only one thread will ever call it)
> >
> > So it's pretty obvious to me that volatile IS NOT good enough if you
> > want a thread-safe solution.
> >
> > [1] http://www.ibm.com/developerworks/java/library/j-jtp06197.html
> > [2]
> http://mina.apache.org/report/1.1/xref/org/apache/mina/common/support/BaseIoSession.html#445
> >
> > regards,
> > Maarten
> >
> > On Wed, May 14, 2008 at 7:44 PM, David M. Lloyd <[EMAIL PROTECTED]>
> wrote:
> >
> > > On 05/14/2008 12:22 PM, Emmanuel Lecharny wrote:
> > >
> > > >
> > > > David M. Lloyd wrote:
> > > >
> > > > >
> > > > > On 05/14/2008 11:57 AM, Emmanuel Lecharny wrote:
> > > > >
> > > > > >
> > > > > > David M. Lloyd wrote:
> > > > > >
> > > > > > >
> > > > > > > On 05/14/2008 11:20 AM, Emmanuel Lecharny wrote:
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > What I can tell at least though is that the session
> configuration
> > > > > > > > > properties provided by IoService should be volatile, because
> they are
> > > > > > > > > accessed in a different thread (I/O processor) almost
> always.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Use synchronization, not volatile. it's too dangerous. When we
> are
> > > > > > > > sure that it works fine, we _may_ switch to volatile.
> > > > > > > >
> > > > > > >
> > > > > > > I don't understand this? Volatile isn't dangerous at all. It's
> > > > > > > clearly defined and well-specified, and using it to make fields
> have
> > > > > > > multi-thread visibility is a perfectly fine usage for them.
> > > > > > >
> > > > > >
> > > > > > The problem is not that volatile is dangerous by itself. Its
> semantic is
> > > > > > really clear, assuming that you _know_ what you can and can't do.
> This is
> > > > > > what scared me : mis-used of volatile.
> > > > > >
> > > > > > Before of using it, I would rather prefer that we expose the clear
> > > > > > benefits we will get from it (and please, bring the numbers with
> you ;)
> > > > > > before using it all over the code.
> > > > > >
> > > > >
> > > > > Well, if you don't use it, then when someone calls a setter on the
> > > > > property, the change may not be propagated to any other threads,
> meaning
> > > > > that different threads could read different values for those
> properties. If
> > > > > you do add volatile, then this doesn't happen - a change in one
> thread is
> > > > > visible immediately thereafter from another thread.
> > > > >
> > > >
> > > > Synchronise the access to this property.
> > > >
> > >
> > > Why? Synchronization is not needed.
> > >
> > >
> > > > Synchronized getter and setter should be ok. Of course, you may have
> to
> > > > copy the property before sending it to the user. Using volatile does
> not
> > > > protect more than synchronize, so I see no reason to use it in this
> case.
> > > >
> > >
> > > The values don't need more protection.
> > >
> > >
> > > > You will have the very same problem in both case, except that volatile
> is
> > > > only meant to protect the visibility.
> > > >
> > >
> > > Visibility is the only problem here that needs to be solved.
> > >
> > >
> > >
> > > > The fact that volatile does not guarantee atomicity is enough a risk
> to
> > > > justify my assertion.
> > > >
> > >
> > > I don't understand. Atomicity is not a requirement here! The only
> > > requirement is that when the values are changed, the changes are
> immediately
> > > visible to other threads. Volatile solves this issue, and in fact was
> > > designed for this. What additional semantics does synchronization
> provide
> > > that are not solved by volatile?
> > >
> > >
> > > > The 'performance' word was used in Trustin's answer to Dmitry mail. If
> it
> > > > does not work, then first synchronize the code in order to make it
> thread
> > > > safe. Volatile is just a weaker way to 'fix the code', not a solution.
> > > >
> > > > Dmitry first mail was about the problem he founds, and he suggested to
> use
> > > > volatile. I suggest we don't and that we synchronize first, until we
> have
> > > > fixed the problem.
> > > >
> > >
> > > This makes no sense. Fear is not a reason to use or not use something.
> The
> > > problem is the lack of visibility, and the solution is volatile.
> > >
> > > - DML
> > >
> > >
> >
>
>
>
>
> --
> Trustin Lee - Principal Software Engineer, JBoss, Red Hat
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
>
