11

I use following code for guarantee startTime variable set once only:

public class Processor
{
    private Date startTime;

    public void doProcess()
    {
        if(startTime == null)
            synchronized(this)
            {
                  if(startTime == null)
                  {
                     startTime = new Date();
                  }
            }

        // do somethings
    }
}

I will guarantee by this code for variable instantiated once only for any number of invoking process method call.

My question is:

Is there alternative approach for my code be more concise? (for sample remove if & synchronized statements)

Sam
  • 6,770
  • 7
  • 50
  • 91
  • I think you can use atomic reference: http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/atomic/AtomicReference.html – Suzan Cioc Jul 25 '12 at 12:22

6 Answers6

12

Use AtomicReference:

public class Processor {
  private final AtomicReference<Date> startTime = new AtomicReference<Date>();
  public void doProcess() {
    if (this.startTime.compareAndSet(null, new Date())) {
      // do something first time only
    }
    // do somethings
  }
}
yegor256
  • 102,010
  • 123
  • 446
  • 597
  • This wastes a `Date` instance for every call, it can be improved. – Marko Topolnik Jul 25 '12 at 12:25
  • This doesn't do the same thing as the OP's code. He wants to initialize `startTime` *only* the first time `doProcess()` is called. – Alex D Jul 25 '12 at 12:36
  • @yegor256 I don't understand what is it work? I know `getAndSet` method return old value and then set new value to passed it. – Sam Jul 25 '12 at 12:47
  • @yegor256 I understand your suggestion,thanks but still it create a `Date` object per invoking `doProcess` method. – Sam Jul 25 '12 at 12:51
  • instantiation of `Date` is a cheap operation, see how the class is implemented in JDK: it has very small memory footprint – yegor256 Jul 25 '12 at 13:06
  • @yegor256 You're right, but we must trying write idea code. In my application maybe `doProcess` triggers 100 time in one minute so I have to find best approach. – Sam Jul 25 '12 at 13:21
  • 1
    you can change the if block to `if((startTime.get() == null) && startTime.compareAndSet(null, new Date()))` – jtahlborn Jul 25 '12 at 13:58
11

Based on you comments, you could use AtomicReference

firstStartTime.compareAndSet(null, new Date());

or AtomicLong

firstStartTime.compareAndSet(0L, System.currentTimeMillis());

I would use

private final Date startTime = new Date();

or

private final long startTime = System.currentTimeMillis();
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 1
    The idea seems to be to trigger the startTime when `doProcess` is called... So I suppose you are suggesting to start the timer when a `Processor` object is created. – assylias Jul 25 '12 at 12:21
  • But in this case startTime will contains timestamp when class was loaded, not when method was called first time? – dbf Jul 25 '12 at 12:22
  • @Peter Lawrey Thanks, but your approach has `final` keyword and must initialized in definition time or in constructor, my sample is not in constructor. – Sam Jul 25 '12 at 12:25
  • @MJM This is true. How many milli-seconds delay do you expect between the object being created and doProcess called for the first time? – Peter Lawrey Jul 25 '12 at 12:27
  • @PeterLawrey Object created in start up time of application but `doProcess` called when a transaction incoming to application(when end-user do a specified action) – Sam Jul 25 '12 at 12:45
  • So its the first time a user triggers the process. If its triggered again, you don't want it to change? – Peter Lawrey Jul 25 '12 at 12:52
  • @PeterLawrey No, I want this variable initialized in first user triggered the process. – Sam Jul 25 '12 at 12:58
  • So its really the `firstStartTime` I would have considered the `startTime` to be the start of the last run. – Peter Lawrey Jul 25 '12 at 13:08
  • 1
    +1 This is the perfect fit for OP's requirement. Proper semantics, no object waste, full speed. – Marko Topolnik Jul 25 '12 at 13:28
  • @MarkoTopolnik "This is the perfect fit for OP's requirement" really but you say "no object waste", this is wrong, in specified status a `Date` instance waste, when tow threads first time go to after `if` statement ;) – Sam Jul 25 '12 at 13:36
  • 1
    MJM, you won't get any better without locking. If there was something **very, very** heavy involved, say a **million** times heavier than a `Date`, you'd realistically need something more. In that case you should read about the proper way to implement the double-check idiom, for example [here](http://java.sun.com/developer/technicalArticles/Interviews/bloch_effective_08_qa.html). It's straight from Josh Bloch's mouth. For your requirement that idiom is **not** a perfect fit for being an overkill by many orders of magnitude. In fact, with 100 calls per minute, even this is overkill. – Marko Topolnik Jul 25 '12 at 13:40
  • Date takes about 200 ns. If you have multiple threads trying to update `firstStartTime` in the same 200 ns, you will get a wasted object producing about 16-24 bytes of garbage. Given this is very rare and very little impact, you may prefer the make the solution as simpler. – Peter Lawrey Jul 25 '12 at 13:46
  • @PeterLawrey I have another question: Is your answer and `Alex D` answer same result? – Sam Jul 25 '12 at 14:25
  • @PeterLawrey curious, in your AtomicReference/Long suggestion, why would the first get() call be needed? Maybe there is a slight performance gain there, but may not be a meaningful optimization? – sjlee Jun 21 '13 at 15:28
  • @sjlee It might be more efficient, but I don't know for sure it is. One line is simpler so I have changed it. – Peter Lawrey Jun 21 '13 at 15:32
4

Your code is an example of so called "double check locking." Please read this article. It explains why this trick does not work in java although it is very smart.

AlexR
  • 114,158
  • 16
  • 130
  • 208
  • Brien Goetz is very smart, but that article is 11 years old. The JVM has certainly changed a great deal since then. I'd wonder if there was a more recent article that dealt with JVM 6 or newer. – duffymo Jul 25 '12 at 12:21
  • 1
    @duffymo The semantics have not changed and this idiom is still a failure. The only relevant change was the fixed semantics of `volatile` that happened with Java 1.5. `volatile` is BTW exactly what OP needs to fix this. – Marko Topolnik Jul 25 '12 at 12:24
3

So from my understanding you need a singleton which is:

  1. Short, easy to implement/understand.
  2. Only initialized when doProcess is called.

I suggest the following implementation using a nested class:

public class Processor {
    private Date startTime;

    private static class Nested {
        public static final Date date = new Date();
    }

    public void doProcess() {
        startTime = Nested.date; // initialized on first reference
        // do somethings
    }
}
Tudor
  • 61,523
  • 12
  • 102
  • 142
  • Your answer is nice but I want decrease my LOC(line of code) ;) I up-vote to this. – Sam Jul 25 '12 at 12:55
2

To sum up what other posters have already explained:

private volatile Date startTime;

public void doProcess()
{
   if(startTime == null) startTime = new Date();
   // ...
}

Concise enough for you?

Alex D
  • 29,755
  • 7
  • 80
  • 126
  • @Alex I wrote your suggestion first but thinking over it and found it isn't thread-safe. – Sam Jul 25 '12 at 12:50
  • The only danger is that multiple threads could initialize `startTime` redundantly, which wouldn't cause anything bad to happen. – Alex D Jul 25 '12 at 12:52
  • 1
    BTW, `java.lang.String` uses the same strategy for lazily initializing its cached hash value. If the above code is "not thread-safe", that means `java.lang.String` is also "not thread-safe". – Alex D Jul 26 '12 at 03:24
0

1 What you have used is known as double checked locking.

2. There are another 2 ways to do it

  - Use synchronized on the Method
  - Initialize the static variable during declaration.

3. As you want an example with No if and synchronized keyword, i am showing you the Initialize the static variable during declaration. way.

public class MyClass{

  private static MyClass unique = new MyClass();

  private MyClass{}

  public static MyClass getInstance(){

      return unique;

  }

 }
Kumar Vivek Mitra
  • 33,294
  • 6
  • 48
  • 75