Just one more thought...
In lines:
358 } else if (stale < 0 && k.get() == null) {
359 stale = i; // remember 1st stale entry index
360 }
...an index to 1st stale entry is remembered while scanning the array so
that at the end, instead of always allocating new slot, an existing slot
can be re-used. This has an inadvertent effect on the lifetime of
SoftReference(s) that are "touched" in the process (LRU policy of
cleaning). Here we see that an additional method like
SoftReference.isPresent() that didn't "touch" the access time, would be
helpful.
Regards, Peter
On 12/06/2014 01:30 PM, Peter Levart wrote:
(Sorry for a re-send, forgot to include core-libs-dev)...
Hi Vladimir,
First, just a nit. I think that in LambdaFormEditor:
289 private LambdaForm putInCache(Transform key, LambdaForm form) {
290 key = key.withResult(form);
291 for (int pass = 0; ; pass++) {
292 Object c = lambdaForm.transformCache;
293 if (c instanceof ConcurrentHashMap) {
294 @SuppressWarnings("unchecked")
295 ConcurrentHashMap<Transform,Transform> m =
(ConcurrentHashMap<Transform,Transform>) c;
296 Transform k = m.putIfAbsent(key, key);
297 if (k == null) return form;
298 LambdaForm result = k.get();
299 if (result != null) {
300 return result;
301 } else {
302 if (m.replace(key, k, key)) {
303 return form;
304 } else {
305 continue;
306 }
307 }
308 }
309 assert(pass == 0);
310 synchronized (lambdaForm) {
311 c = lambdaForm.transformCache;
312 if (c instanceof ConcurrentHashMap)
313 continue;
...
372 lambdaForm.transformCache = c = m;
^^^ put
assignment to 'c' back in
373 // The second iteration will update for this
query, concurrently.
374 continue;
...you could move the assignment to 'c' in line 292 out of for loop
and put it back in line 372, since once 'c' is instance of CHM,
lambdaForm.transformCache never changes again and if 'c' is not CHM
yet, it is re-assigned in lines 311 and 372 before next loop.
Am I right?
Now what scares me (might be that I don't have an intimacy with
LambdaForm class like you do). There is a situation where you publish
LambdaForm instances via data race.
One form of LambdaForm.transformCache is an array of Transform objects
(the other two forms are not problematic). Transform class has all
fields final except the 'referent' field of SoftReference, which holds
a LambdaForm instance. In the following line:
377 ta[idx] = key;
...you publish Transform object to an element of array with relaxed
write, and in the following lines:
271 } else {
272 Transform[] ta = (Transform[])c;
273 for (int i = 0; i < ta.length; i++) {
274 Transform t = ta[i];
275 if (t == null) break;
276 if (t.equals(key)) { k = t; break; }
277 }
278 }
279 assert(k == null || key.equals(k));
280 return (k != null) ? k.get() : null;
...you obtain the element of the array with no synchronization and a
relaxed read and might return a non-null referent (the LambdaForm)
which is then returned as an interned instance.
So can LambdaForm instances be published via data races without fear
that they would appear half-initialized?
That's what I didn't know when I used a lazySet coupled with volatile
get to access array elements in my version:
http://cr.openjdk.java.net/~plevart/misc/LambdaFormEditor.WeakCache/webrev.01/
Regards, Peter
On 12/03/2014 12:45 PM, Vladimir Ivanov wrote:
Aleksey, thanks for the review.
I haven't tried -XX:SoftRefLRUPolicyMSPerMB=0, but I did extensive
testing on Octane/Nashorn with multiple low -Xmx levels + frequent
Full GCs (8060147 [1] was the result of those experiments) and stress
tested cache eviction with jdk/java/lang/invoke/LFCache tests in long
running mode.
Best regards,
Vladimir Ivanov
[1] https://bugs.openjdk.java.net/browse/JDK-8060147
On 12/3/14, 3:11 PM, Aleksey Shipilev wrote:
On 12/01/2014 07:58 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057020/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057020
Looks okay, although the cache management logic gives me a headache
after the vacation. I thought I spotted a few bugs, but those were only
false positives.
The fix is to use SoftReferences to keep LambdaForms alive as long as
possible, but avoid throwing OOME until the caches are evicted. I
experimented with WeakReferences, but it doesn't hold LambdaForms for
long enough: LambdaForm cache hit rate degrades significantly and it
negatively affects application startup and warmup, since every
instantiated LambdaForm is precompiled to bytecode before usage.
Testing: jdk/java/lang/invoke/LFCache in stress mode + jck
(api/java_lang/invoke), jdk/java/lang/invoke,
jdk/java/util/streams, octane
SoftReferences are tricky in the way they can get suddenly drop the
referent, and normal testing would not catch it (e.g. the normal
operation would reclaim softrefs under your feet almost never). Does
this code survive with -XX:SoftRefLRUPolicyMSPerMB=0?
Thanks,
-Aleksey.
_______________________________________________
mlvm-dev mailing list
mlvm-...@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
