1

I've been messing around with NetBeans creating a clock that is always on top and always loads in the bottom right of the screen. I've gotten it completed however I think my code is eating away at memory, after I left it over night I came back to 1.4GB of memory being used and a large amount of the CPU being used. I'm new to Java programming so I'm hoping this isn't normal!

In my main thread I create a new Calendar object each run. Moving this out of the Thread creates a Calendar object that is initialised with the current time but never updates, is there a better method of doing this?

I'm not used to dealing with Threads and I think I may have gotten turned around. Can anyone suggest improvements to the following that will lower my memory footprint and cpu usage?

public JavaClockGUI() {
    initComponents();    

    new Thread()
    {
        public void run()
        {
            while(true) 
            {
                Calendar cal = new GregorianCalendar();
                int hour = cal.get(Calendar.HOUR_OF_DAY);
                int min = cal.get(Calendar.MINUTE);
                int sec = cal.get(Calendar.SECOND);

                String time = String.format("%02d",hour) + ":" + String.format("%02d",min) + ":" + String.format("%02d",sec);

                lblTime.setText(time);
            }
        }
    }.start();
}
Otakie
  • 13
  • 4
  • Try replace Thread by TimerTask – Thanh Le May 07 '15 at 08:35
  • 2
    Swing Timer is simpler of possible ways – mKorbel May 07 '15 at 08:38
  • Based on the snipper execute the `while-loop` in the `run()` method as fast as the CPU can serve it. This is not really needed, as the seconds change not that often. ;-) Or do you miss to post the code which is periodically called? – SubOptimal May 07 '15 at 08:38
  • @SubOptimal I think thats the main issue with my code, I'm running it as fast as possible. Unfortunately I haven't missed that bit out. – Otakie May 07 '15 at 08:46
  • Thanks for the other suggestions too I'll do a little research now. – Otakie May 07 '15 at 08:48
  • Calling `setText` on a `JLabel` from a background thread is invalid. See [“Swing’s Threading Policy”](http://docs.oracle.com/javase/8/docs/api/javax/swing/package-summary.html#threading). That’s one of the reasons why using a [Swing Timer](http://docs.oracle.com/javase/8/docs/api/javax/swing/Timer.html) is much easier… – Holger May 07 '15 at 11:25

3 Answers3

0

try declaring the variables outside the while loop and having a delay to lessen execution of the loop on a given amount of time.

public void run()
    {
        Calendar cal;
        int hour;
        int min;
        int sec;
        String time;
        while(true) 
        {
            cal = new GregorianCalendar();
            hour = cal.get(Calendar.HOUR_OF_DAY);
            min = cal.get(Calendar.MINUTE);
            sec = cal.get(Calendar.SECOND);

            String time = String.format("%02d",hour) + ":" +  String.format("%02d",min) + ":" + String.format("%02d",sec);

            lblTime.setText(time);
            //delay the loop for 1 sec
            try {
                Thread.currentThread().sleep(1000);
                System.out.println(x);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
        }
    }
  • Where do I define the Thread? My current method defines the Thread as wrapping run so I can't sleep the Thread while inside it. I don't think... – Otakie May 07 '15 at 09:16
0

Hope this will help you.

Date(){
    DateFormat dateFormat = new SimpleDateFormat("MMM dd, yyyy HH:mm:ss)";
    Date date = new Date();
    boolean condition = false;

    while(!false){
      lbltime.setText(dateFormat.format(date));
      Thread.sleep(1000);
   }
}
Rich
  • 3,928
  • 4
  • 37
  • 66
0

Two important things about performance:

  • Not to create a new object inside the loop every iteration (and avoid heap allocation as much as possible)
  • Execute the task periodically (and not let a while(true) loop run rampant).

I don't know what's the best way to do it, but I have a short, simple and accurate one:

Use a Timer to execute a TimerTask at a fixed rate. The method scheduleAtFixedRate will execute the task (run method) at a fixed rate with respect to absolute time, and not with respect to previous execution. That is, if one of the executions is delayed, the next one will commence regardless of the delay (they will be shorter apart). Calling Thread.sleep(1000) will not behave this way and will result in accumulating delays in the long run.

For receiving the time and formatting it, the Java8 Date-Time API (java.time) is the way to go.

public class Example {

    public static void main(String[] args) {

        DateTimeFormatter formatter = DateTimeFormatter.ofLocalizedTime(FormatStyle.MEDIUM);
        new Timer().scheduleAtFixedRate(new TimerTask() {

            @Override
            public void run() {

                System.out.println(LocalTime.now().format(formatter));
            }
        }, 0, 1000);
    }
}

Notes:

  • You can make your class extend TimerTaks if you don't like the anonymous construction here, or create an inner class.
  • Explore DateTimeFormatter to find the format you want if this one is not what you're looking for.
  • You can hold a reference to timer,

    Timer timer = new Timer();
    timer.scheduleAtFixedRate(...);
    

    If you need it later.

user1803551
  • 12,965
  • 5
  • 47
  • 74
  • Ahh yes I think that's what I need. I tried it and I've dropped all the way to 25mb which I imagine is the compiler overhead. The only issue I have now is that I am trying to run `lblText.setText(LocalTime.now().format(formatter))` inside static void main and I get a non-static variable static context error. Is there a simple way around it? – Otakie May 07 '15 at 12:36
  • The issue with `static` is a basic concept in Java related to the overall design of your program. It is also unrelated to this issue. See maybe [this](http://stackoverflow.com/questions/4969171/cannot-make-static-reference-to-non-static-method) or [this](http://stackoverflow.com/questions/8101585/cannot-make-a-static-reference-to-the-non-static-field). You probably need to make your label static. – user1803551 May 07 '15 at 12:45
  • will do thanks for letting me know. I tried upvoting but I don't have the rep yet. Also I wasn't sure whether to accept it as answered as I was still stuck on the very end part. Thanks for letting me know. – Otakie May 07 '15 at 12:51
  • If you're stuck with the compiler error about `static` then post a new question with a [minimal code that we can run and that demonstrates the problem](http://stackoverflow.com/help/mcve). If you need clarification or help about the issue in this page then ask. – user1803551 May 07 '15 at 12:58