Fwd: RFR (XXS): JDK-8046668 : Excessive checked JNI warnings from Java startup

[David Holmes]

On 11/07/2014 5:37 PM, David Simms wrote:
> Hi David,
>
> You mean the "jni.cpp:jni_functions()" which returns the checked
> functions (with -Xcheck:jni flag set) in the "JNINativeInterface" struct
> of function pointers ?

Yes. So without -Xcheck:jni we call the functions in jni.cpp else we 
call the ones in jniChecked.cpp - okay.

> I recently add these extra warnings, if a function is call without
> previously checking for exceptions.

Ah I see.

> As stated in JDK-8046668 warnings for not checking functions that throw
> ArrayOutOfBoundException aren't very useful, since most code makes
> length and offset checks before calling, see the bug for an example. "
> set_pending_jni_exception_check" arms the need for an exception check
> before calling further JNI functions.
>
> https://bugs.openjdk.java.net/browse/JDK-8046668

I can almost buy that for the index checks (though I'm not sure it's 
really up to us to decide which checks the user would want and which 
they would not [ideally the user would be able to control that]).

But some of these can also throw ArrayStoreException so I imagine, as 
per the example that this kind of construct:

     if ((*env)->ExceptionCheck(env)) {
           assert(0 && "should not reach here");

would actually be quite common to validate expectations.

But this is all subjective. :)

Thanks for clarifying.

David

> Hope this was the answer you wanted
> /Mr. Simms
>
> On 2014-07-11 09:17, David Holmes wrote:
>> Hi David,
>>
>> Sorry I'm having trouble connecting the dots here. How do we get from
>> the JNI entry points defined in jni.cpp these "checked "entry points
>> in jniCheck.cpp ? I can't reconcile how the warnings can be removed
>> when some of these functions are used.
>>
>> Thanks,
>> David H.
>>
>> On 11/07/2014 4:19 PM, David Simms wrote:
>>>
>>> Thanks for the review Harold and Lois,
>>>
>>> I've updated the patch with your comments:
>>>
>>> http://cr.openjdk.java.net/~dsimms/8046668/
>>>
>>> Thanks
>>> /David Simms
>>>
>>> On 2014-07-10 14:16, Lois Foltan wrote:
>>>>
>>>> Hi David,
>>>>
>>>> Looks good, given the number of exeception_checks you removed I agree
>>>> with your description of pedantic :-) !
>>>>
>>>> src/share/vm/prims/jniCheck.cpp
>>>>   - subjective comment on my part, I find the name of the new method a
>>>> bit long, would
>>>>     you consider renaming to "add_planned_handle_capacity"?
>>>>  -  Also, would you consider within the function just calling
>>>> set_planned_capacity(capacity +
>>>>     handles->get_number_of_live_handles() +
>>>> CHECK_JNI_LOCAL_REF_CAP_WARN_THRESHOLD)?
>>>>     Modifying a parameter that was passed in by value can be confusing.
>>>>
>>>> Overall, though I am good with the change, reviewed.
>>>>
>>>> Thanks,
>>>> Lois
>>>>
>>>>
>>>>>
>>>>> -------- Original Message --------
>>>>> Subject:     RFR (XXS): JDK-8046668 : Excessive checked JNI warnings
>>>>> from Java startup
>>>>> Date:     Tue, 17 Jun 2014 10:26:10 +0200
>>>>> From:     David Simms <david.simms at oracle.com>
>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>> <hotspot-runtime-dev at openjdk.java.net>
>>>>>
>>>>>
>>>>>
>>>>> Hello,
>>>>>
>>>>> I have fix for the following bug:
>>>>>
>>>>>      JDK-8046668 : "Excessive checked JNI warnings from Java startup"
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8046668
>>>>>
>>>>> Web review:
>>>>>
>>>>> http://cr.openjdk.java.net/~dsimms/8046668/
>>>>>
>>>>> This fix remove some of the pedantic JNI exception checks warnings for
>>>>> functions that throw ArrayOutOfBoundException. It was found almost all
>>>>> code checks array length, making exception checking code
>>>>> meaningless for
>>>>> these function.
>>>>>
>>>>> Also fix a small bug for the excessive local reference warnings
>>>>> (always
>>>>> add to current number of live handles).
>>>>>
>>>>> Testing:
>>>>>
>>>>>      All platforms:
>>>>>      - JPRT
>>>>>      - VM testbase: "  vm.quick.testlist
>>>>> vm.parallel_class_loading.testlist vm.regression.testlist
>>>>> vm.runtime.testlist"
>>>>>      - JTREG: JDK & JVM
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>