[PATCH] tests for VmCpuStatBuilder

Omair Majid omajid at redhat.com
Wed Mar 28 18:42:12 UTC 2012


On 03/28/2012 01:52 PM, Jon VanAlten wrote:
> From: "Omair Majid" <omajid at redhat.com>
>> 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?

I didn't really think about it. It's only used in the system backend so
I kept it there for now. I will move it over when we make other places
use Clock too. Or I can move it right now if you think it's a good idea.

> +    @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).

Yeah, but other than overflow, nanoTime() isn't going to go back in
time. On overflow, the difference (what we really care about with
monotonic clocks) will wrap over properly too. Only samples taken more
than ~300 years apart are going to cause problems [1]. For all intents
and purposes this is monotonic. Whereas currentTimeMillis() will change
every so often (thanks to ntp, daylight savings, and any timezone
differences).

As for java getting such a thing: no, I don't anticipate java getting
this anytime soon. Although hotspot itself uses many of the CLOCK_*
clocks that you can use with clock_gettime(2). Pity it isn't exposed in
the runtime :(

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

I can leave this method out completely; I am not using it in the code.
It was a part of the TODO - I thought it might be nice to have this
method at some point. Currently wrappers for  currentTimeMillis() and
nanoTime() are sufficient.

>> 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?
> 

I guess I will leave it for now.

Cheers,
Omair

[1]
http://docs.oracle.com/javase/7/docs/api/java/lang/System.html#nanoTime%28%29



More information about the Thermostat mailing list