[infinispan-dev] Time measurement and expiry

[Sanne Grinovero]

On 17 October 2011 09:02, Dan Berindei <dan.berindei at gmail.com> wrote:
> On Sat, Oct 15, 2011 at 7:25 PM, Sanne Grinovero <sanne at infinispan.org> wrote:
>> Hi all,
>> I'm looking at some performance details, and noticed the current code
>> is using System.currentTimeMillis() to estimate performance for short
>> lived operations.
>>
>> For the purpose of CacheMgmtInterceptor we should use System.nanoTime() instead:
>> http://blogs.oracle.com/dholmes/entry/inside_the_hotspot_vm_clocks
>>
>
> Very interesting, I knew that in Windows currentTimeMillis() basically
> just reads a volatile because I got bit by the 15 millisecond accuracy
> issue before, so I thought it would always be very fast. I had no idea
> on Linux it would have the same performance as nanoTime().

Even in Windows it seems to be a bit more expensive than just reading
a volatile;

>> Searching for "currentTimeMillis()" in the code base reveals it's
>> being used for expiry of entries as well. If someone messes with
>> system clocks data will expire; is it expected that entries expire at
>> calendar time and not at a set lifetime?
>
> I would say if someone's messing with system clocks they already have
> a much bigger problem than their cached data expiring...

Absolutely, but this is just an example. I'd expect that if I'm using
it as a cache, the data will stay in it for a certain amount of
relative time.
It doesn't have to be a user changing the clock, for example there
might be automated changes like daylight adjustments (summer/winter
time) or relatively small updates via NTP to keep the clocks in sync..
of course they might not affect the functionality significantly, but
I'm just pointing out that we might prefer to use the nanoTime source
as our API suggests a period of entry validity in milliseconds, not an
expiry in terms of calendar Date.

> Besides, someone may intentionally change the system time to test the
> ISPN expiration algorithm :)
>
>> I understand we need to use the absolute time to be able to transmit
>> the value to other nodes, where the nanoTime of different nodes might
>> not be comparable, but we could store one and use both: transmit the
>> absolute value only to other nodes and refer to the other for accurate
>> expiries.
>> Other nodes receiving the absolute value will check for the remaining
>> lifespane and store the corresponding nanoTime.
>>
>> ExpiryHelper itself will invoke the currentTimeMillis() operation
>> internally, that means that it's going to be invoked at least once for
>> each entry being accessed and might result in a lot of invocations
>> when traversing several entries; I'm wondering if it shouldn't take a
>> millisecond parameter to consider as current, so that this relatively
>> expensive method can be invoked only once at the beginning of a batch
>> of operations.
>>
>
> + 1 for adding a currentTime parameter to the isExpiredX() methods in
> ExpiryHelper and also to isExpired() CacheEntry and its
> implementations.

Agreed: ISPN-1459

>> Also reading this time information is a high overhead in some
>> configurations, I'm wondering if we should make it possible to
>> configure Infinispan to not track performance on each cache operation?
>> Someone might prefer to estimate an average from multiple calls; I'm
>> going to remove CacheMgmtInterceptor for my tests.
>>
>
> CacheMgmtInterceptor doesn't seem to be enabled when JMX statistics
> are disabled, so I don't think you need to explicitly remove
> CacheMgmtInterceptor for your performance tests.

Yes that's how I do it ;)
What concerns me is that there is going to be a relatively strong
overhead just by enabling statistics.

>
> Cheers
> Dan