Good call. I'm so used to making code thread-safe in a non-performance critical enterprise where not many programmers understand concurrency at all. I guess I need to update my IntelliJ inspection settings to modernise them a bit!
On 19 May 2014 02:25, Remko Popma <[email protected]> wrote: > 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]> > > -- Matt Sicker <[email protected]>
