On Sat, Oct 15, 2011 at 7:25 PM, Sanne Grinovero <sanne(a)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().
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...
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.
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.
Cheers
Dan