[PATCH] tests for VmCpuStatBuilder

Jon VanAlten jvanalte at redhat.com
Wed Mar 28 17:52:59 UTC 2012



----- Original Message -----
> From: "Omair Majid" <omajid at redhat.com>
> To: "Jon VanAlten" <jvanalte at redhat.com>
> Cc: thermostat at icedtea.classpath.org
> Sent: Wednesday, March 28, 2012 1:03:59 PM
> Subject: Re: [PATCH] tests for VmCpuStatBuilder
> 
> On 03/27/2012 08:41 PM, Jon VanAlten wrote:
> > From: "Omair Majid" <omajid at redhat.com>
> >>
> >> The attached patch adds tests for VmCpuStatBuilder.
> >>
> > This looks good.  Nice idea for testing stuff that depends
> > on time.  Maybe it would be even better to generalize it
> > right away?
> 
> Done, with the caveat that Java doesn't actually provide us with
> anything (as far as I know) to help us get lower resolution monotonic
> (or non-monotonic, aka real) times. So I have a dummy implementation
> there for now.
> 

Cool.  Any particular reason why you kept it part of system backend
package, rather than moving it over to somewhere in common?

+    @Override
+    public long getMonotonicTimeMillis() {
+        // this is supposed to be an inexpensive version of a monotonic clock.
+        // no such thing in the java api currently, so let's fake it for now.
+        return TimeUnit.NANOSECONDS.toMillis(System.nanoTime());
+    }

If cost is your concern, why not a hack based on currentTimeMillis() rather
than nanoTime()?  Do you anticipate a coarser monotonic clock enhancement
to some future version of Java?  (note that nanoTime is not guaranteed
monotonic either, it can overflow between calls).

If you really think nanoTime is the better way, then don't let my questions
block this :)

> > Another thing I'd ask about that is to please
> > make it clear right from the method name that the different
> > methods return time in different units.
> 
> Done.
> 
> A sort-of follow-up to this change would be making all the other code
> calling System.currentTimeMillis() or System.nanoTime() use this
> instead. Would that be worth doing? If so, how does modifying
> ApplicationContext to return a clock sound? Or should I just pass the
> Clock object around?
> 

I would say to make the change where we want to mock a Clock for
testing, but not make a great effort to make the change project-wide.
Unless you have some other benefit in mind (besides testing) for the
change?

cheers,
jon



More information about the Thermostat mailing list