RFR: 8193222: EnsureLocalCapacity() should maintain capacity requests through multiple calls

[Daniel D. Daugherty]

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
>