[9, 8u40] RFR (M): 8057020: LambdaForm caches should support eviction
Peter Levart
peter.levart at gmail.com
Mon Dec 8 10:25:21 UTC 2014
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-dev at openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
>
More information about the mlvm-dev
mailing list