RFR: 8193222: EnsureLocalCapacity() should maintain capacity requests through multiple calls
David Holmes
david.holmes at oracle.com
Tue Dec 12 01:18:53 UTC 2017
Thanks Dan! Nits will be fixed.
David
On 12/12/2017 11:00 AM, Daniel D. Daugherty wrote:
> On 12/10/17 12:39 AM, David Holmes wrote:
>> bug: https://bugs.openjdk.java.net/browse/JDK-8193222
>> webrev: http://cr.openjdk.java.net/~dholmes/8193222/webrev/
>
> make/test/JtregNativeHotspot.gmk
> No comments.
>
> src/hotspot/share/prims/jniCheck.cpp
> No comments.
>
> test/hotspot/jtreg/runtime/jni/checked/TestCheckedEnsureLocalCapacity.java
> L42: // If copies > capacity + warning-threshold then we still
> get a warning
> Nit: Needs a period at the end.
>
> L75: //Warning
> Nit: Needs a space before 'W'.
>
> test/hotspot/jtreg/runtime/jni/checked/libTestCheckedEnsureLocalCapacity.c
> No comment.
>
> Thumbs up! Your choice on whether to fix the nits.
> Don't need to see another webrev even if you do...
>
> Dan
>
>
>>
>> In product mode, for hotspot, EnsureLocalCapacity is a no-op as there
>> is no artificial limit applied for local refs.
>>
>> With -Xcheck:jni the checked mode was augmented to watch for
>> "excessive" use of local refs and to produce a warning if that
>> happened e.g.:
>>
>> WARNING: JNI local refs: 41, exceeds capacity: 40
>>
>> That was added under JDK-8043224.
>>
>> The problem is that the code always modifies the planned capacity
>> (that expected before warnings should be used) without regard for the
>> fact the existing capacity may be higher than that requested. As a
>> result if you have a call chain like:
>>
>> void foo() { // C code
>> env->EnsureLocalCapacity(60); // needs lots of local refs
>> ...
>> JNU_GetPlatformsString(...)
>> env->EnsureLocalCapacity(5); // lower than 60!
>> ...
>> // create 60 local refs
>> }
>>
>> upon return the warning will be issued because the number of local
>> refs exceeds the most recent call to EnsureLocalCapacity.
>>
>> A simple fix is for EnsureLocalCapacity to only raise the planned
>> capacity, not lower it. That fits with the notion of "ensuring" there
>> is sufficient space - the function is not SetLocalcapacity. It also
>> fits with the way PushLocalFrame(capacity) increases the planned
>> capacity by "capacity" but PopLocalFrame does not reduce it again.
>>
>> New test added.
>>
>> Tested through JPRT.
>>
>> Thanks,
>> David
>>
>
More information about the hotspot-runtime-dev
mailing list