I saw you reverted the change. Thanks! Take a look at the implementation for AtomicLong get() and set(). They just set the value of the volatile long field of the AtomicLong instance. So we're just adding unnecessary method invocations. True, hotspot may inline them, so the result may be the same, but I just didn't see the point.
(AtomicLong is needed for compareAndSet stuff though.) Sent from my iPhone > On 2014/05/19, at 13:26, Matt Sicker <[email protected]> wrote: > > I'm going to revert it, but I have a feeling that AtomicLong takes less than > 1 millisecond to update. Then again, depending on the version of the JVM, > volatile should be faster (and the CPU architecture of course). > > >> On 18 May 2014 23:20, Matt Sicker <[email protected]> wrote: >> Looks like my info here might be out of date actually. >> >> http://stackoverflow.com/questions/3038203/is-there-any-point-in-using-a-volatile-long >> >> This class doesn't necessarily have to be atomic does it? If it's purely for >> performance gains, it doesn't really seem so. >> >> >>> On 18 May 2014 22:45, Remko Popma <[email protected]> wrote: >>> Matt, >>> >>> Sorry, I didn't get that. What is the problem with a plain volatile long? >>> >>> This class is a bit performance sensitive: it can be used instead of the >>> system clock to squeeze the last drop of performance out of async logging, >>> in return for less precision. >>> Did you verify that this implementation is just as fast? >>> >>> Regards, >>> Remko >>> >>> Sent from my iPhone >>> >>> > On 2014/05/19, at 12:18, [email protected] wrote: >>> > >>> > Author: mattsicker >>> > Date: Mon May 19 03:18:09 2014 >>> > New Revision: 1595736 >>> > >>> > URL: http://svn.apache.org/r1595736 >>> > Log: >>> > Use AtomicLong over volatile long due to problematic implementations of >>> > volatile long. >>> > >>> > Modified: >>> > >>> > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/util/CoarseCachedClock.java >>> > >>> > Modified: >>> > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/util/CoarseCachedClock.java >>> > URL: >>> > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/util/CoarseCachedClock.java?rev=1595736&r1=1595735&r2=1595736&view=diff >>> > ============================================================================== >>> > --- >>> > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/util/CoarseCachedClock.java >>> > (original) >>> > +++ >>> > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/util/CoarseCachedClock.java >>> > Mon May 19 03:18:09 2014 >>> > @@ -16,6 +16,7 @@ >>> > */ >>> > package org.apache.logging.log4j.core.util; >>> > >>> > +import java.util.concurrent.atomic.AtomicLong; >>> > import java.util.concurrent.locks.LockSupport; >>> > >>> > /** >>> > @@ -24,14 +25,13 @@ import java.util.concurrent.locks.LockSu >>> > */ >>> > public final class CoarseCachedClock implements Clock { >>> > private static final CoarseCachedClock instance = new >>> > CoarseCachedClock(); >>> > - private volatile long millis = System.currentTimeMillis(); >>> > + private final AtomicLong millis = new >>> > AtomicLong(System.currentTimeMillis()); >>> > >>> > private final Thread updater = new Thread("Clock Updater Thread") { >>> > @Override >>> > public void run() { >>> > while (true) { >>> > - final long time = System.currentTimeMillis(); >>> > - millis = time; >>> > + millis.set(System.currentTimeMillis()); >>> > >>> > // avoid explicit dependency on sun.misc.Util >>> > LockSupport.parkNanos(1000 * 1000); >>> > @@ -62,6 +62,6 @@ public final class CoarseCachedClock imp >>> > */ >>> > @Override >>> > public long currentTimeMillis() { >>> > - return millis; >>> > + return millis.get(); >>> > } >>> > } >>> > >>> > >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: [email protected] >>> For additional commands, e-mail: [email protected] >> >> >> >> -- >> Matt Sicker <[email protected]> > > > > -- > Matt Sicker <[email protected]>
