RFR: 8273563: Improve performance of implicit exceptions with -XX:-OmitStackTraceInFastThrow [v6]

[Volker Simonis]

On Wed, 10 Nov 2021 16:56:07 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Volker Simonis has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add new WhiteBox functionality to sun/hotspot/WhiteBox.java as well to avoid warnings in the tests which are still using it.
>
> src/hotspot/share/prims/whitebox.cpp line 987:
> 
>> 985:     bool overflow = false;
>> 986:     for (uint reason = 0; reason < mdo->trap_reason_limit(); reason++) {
>> 987:       if (reason_str != NULL && !strcmp(reason_str, Deoptimization::trap_reason_name(reason))) {
> 
> Maybe the code would be better readable when checking `reason_str != NULL` first and then use 2 loops? Just a minor suggestion. Should only be done if readability is better.

I've tried it but the resulting version is slightly longer and in my opinion not really more readable:

WB_ENTRY(jint, WB_GetMethodTrapCount(JNIEnv* env, jobject o, jobject method, jstring reason_obj))
  jmethodID jmid = reflected_method_to_jmid(thread, env, method);
  CHECK_JNI_EXCEPTION_(env, 0);
  methodHandle mh(THREAD, Method::checked_resolve_jmethod_id(jmid));
  uint cnt = 0;
  MethodData* mdo = mh->method_data();
  if (mdo != NULL) {
    ResourceMark rm(THREAD);
    if (reason_obj != NULL) {
      char* reason_str = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(reason_obj));
      for (uint reason = 0; reason < mdo->trap_reason_limit(); reason++) {
        if (!strcmp(reason_str, Deoptimization::trap_reason_name(reason))) {
          cnt = mdo->trap_count(reason);
          // Count in the overflow trap count on overflow
          if (cnt == (uint)-1) {
            cnt = mdo->trap_count_limit() + mdo->overflow_trap_count();
          }
          break;
        }
      }
    } else {
      bool overflow = false;
      for (uint reason = 0; reason < mdo->trap_reason_limit(); reason++) {
        uint c = mdo->trap_count(reason);
        if (c == (uint)-1) {
          c = mdo->trap_count_limit();
          if (!overflow) {
            // Count overflow trap count just once
            overflow = true;
            c += mdo->overflow_trap_count();
          }
        }
        cnt += c;
      }
    }
  }
  return cnt;
WB_END


But for me it's actually no difference. Please just let me know if you'd still prefer the alternative version.

PS: I've updated the documentation of the method which was inaccurate for `reason==NULL`.

> src/hotspot/share/prims/whitebox.cpp line 1016:
> 
>> 1014:   }
>> 1015:   ResourceMark rm(THREAD);
>> 1016:   char *reason_str = (reason_obj == NULL) ?
> 
> I think we should use `const char*` as far as possible.

Done.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5488