RFR: 8304138: [JVMCI] Test FailedSpeculation existence before appending. [v3]

[Vladimir Kozlov]

On Thu, 16 Mar 2023 21:19:53 GMT, Yudi Zheng <yzheng at openjdk.org> wrote:

>> Upon uncommon_trap, JVMCI runtime appends a FailedSpeculation entry to the nmethod using an [atomic operation](https://github.com/openjdk/jdk/blob/55aa122462c34d8f4cafa58f4d1f2d900449c83e/src/hotspot/share/oops/methodData.cpp#L852). It becomes a performance bottleneck when there is a large amount of (virtual) threads deoptimizing in the nmethod. In this PR, we test if a FailedSpeculation exists in the list before appending it.
>
> Yudi Zheng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   avoid duplicated entry.

Yes, it would do what you are saying but I don't like infinite loops like that with conditional exists.
How big length of `failed_speculations_address` list you can have?
Consider implementing it as recursive method if depth is not big:

bool FailedSpeculation::add_failed_speculation(nmethod* nm, FailedSpeculation** failed_speculations_address, address speculation, int speculation_len) {
  assert(failed_speculations_address != nullptr, "must be");
  guarantee_failed_speculations_alive(nm, failed_speculations_address);

  size_t fs_size = sizeof(FailedSpeculation) + speculation_len;
  FailedSpeculation* fs = new (fs_size) FailedSpeculation(speculation, speculation_len);
  if (fs == nullptr) {
    // no memory -> ignore failed speculation
    return false;
  }
  guarantee(is_aligned(fs, sizeof(FailedSpeculation*)), "FailedSpeculation objects must be pointer aligned");
  
  if (!add_failed_speculation_recursive(failed_speculations_address, fs)) {
    delete fs;
    return false;
  }
  return true;
}

bool add_failed_speculation_recursive(FailedSpeculation** cursor, FailedSpeculation* fs) {
  if (*cursor == nullptr) {
    FailedSpeculation* old_fs = Atomic::cmpxchg(cursor, (FailedSpeculation*) nullptr, fs);
    if (old_fs == nullptr) {
      // Successfully appended fs to end of the list
      return true;
    }
    guarantee(*cursor != nullptr, "cursor must point to non-null FailedSpeculation");
  }
  // check if the current entry matches this thread's failed speculation
  int speculation_len = fs->data_len();
  if ((*cursor)->data_len() == speculation_len && memcmp(fs->data(), (*cursor)->data(), speculation_len) == 0) {
    return false;
  }
  return add_failed_speculation_recursive((*cursor)->next_adr(), fs);
}

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

PR: https://git.openjdk.org/jdk/pull/13022