Thanks David, Peter for your review and comments. As I am new to the
community, can you please help me to open a bug and integrate the
changes into code base?
-Venkat
On 12/11/17 2:19 AM, Peter Levart wrote:
Hi David, Venkat,
On 11/11/17 21:15, Peter Levart wrote:
For example, take the following method:
String defaultTZID() {
return TimeZone.getDefault().getID();
}
When JIT compiles it and inlines invocations to other methods within
it, it can prove that cloned TimeZone instance never escapes the call
to defaultTZID() and can therefore skip allocating the instance on heap.
But this is fragile. If code in invoked methods changes, they may not
get inlined or EA may not be able to prove that the cloned instance
can't escape and allocation may be introduced. ZoneId.systemDefault()
is a hot method and it would be nice if we manage to keep it
allocation free.
Well, I tried the following variant of SimpleTimeZone.clone() patch:
public Object clone()
{
SimpleTimeZone tz = (SimpleTimeZone) super.clone();
// like tz.invalidateCache() but without holding a lock on clone
tz.cacheYear = tz.startYear - 1;
tz.cacheStart = tz.cacheEnd = 0;
return tz;
}
...and the JMH benchmark with gc profiling shows that
ZoneId.systemDefault() still manages to get JIT-compiled without
introducing allocation.
Even the following (original Venkat's) patch:
public Object clone()
{
SimpleTimeZone tz = (SimpleTimeZone) super.clone();
tz.invalidateCache();
return tz;
}
...does the same and the locking in invalidateCache() is elided too.
Allocation and lock-elision go hand-in-hand. When object doesn't
escape, allocation on heap may be eliminated and locks on that
instance elided.
So this is better than synchronizing on the original instance during
.clone() execution as it has potential to avoid locking overhead.
So Venkat, go ahead. My fear was unjustified.
Regards, Peter
