RFR: JDK-8293313: NMT: Rework MallocLimit [v4]

[Gerard Ziemski]

On Wed, 25 Jan 2023 06:51:04 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/services/mallocTracker.inline.hpp line 56:
>> 
>>> 54:       size_t so_far = as_snapshot()->total();
>>> 55:       if ((so_far + s) > l->sz) { // hit the limit
>>> 56:         if (!suppress_limit_handling()) {
>> 
>> I would prefer to see `suppress_limit_handling()` checked inside `total_limit_reached()`, same for `category_limit_reached()`. This way we would force those APIs to always obey the suppression without having to bother to add `suppress_limit_handling()`, they could call `VMError::is_error_reported()` directly.
>
>> I would prefer to see `suppress_limit_handling()` checked inside `total_limit_reached()`, same for `category_limit_reached()`. This way we would force those APIs to always obey the suppression without having to bother to add `suppress_limit_handling()`, they could call `VMError::is_error_reported()` directly.
> 
> Note that `total_limit_reached()` and `category_limit_reached()` are private APIs. 
> 
> If we move `suppress_limit_handling()` into `total_limit_reached()`, `total_limit_reached()` would need to report it back to `MallocMemorySummary::check_exceeds_limit` since its processed there too, it affects the return code. So `total_limit_reached()` cannot be void anymore, and gets somewhat more complex. I prefer it this way, tbh.

I'm not following.

If we get rid of `suppress_limit_handling()` SPI, and do:


bool MallocMemorySummary::total_limit_reached(size_t s, size_t so_far, const malloclimit* limit) {
  if (VMError::is_error_reported()) {
    return false;
  }

#define FORMATTED \
"MallocLimit: reached global limit (triggering allocation size: " PROPERFMT ", allocated so far: " PROPERFMT ", limit: " PROPERFMT ") ", \
PROPERFMTARGS(s), PROPERFMTARGS(so_far), PROPERFMTARGS(limit->sz)

  if (limit->mode == MallocLimitMode::trigger_fatal) {
    fatal(FORMATTED);
  } else {
    log_warning(nmt)(FORMATTED);
  }

#undef FORMATTED

  return true;
}

bool MallocMemorySummary::category_limit_reached(MEMFLAGS f, size_t s, size_t so_far, const malloclimit* limit) {
  if (VMError::is_error_reported()) {
    return false;
  }

#define FORMATTED \
  "MallocLimit: reached category "%s" limit (triggering allocation size: " PROPERFMT ", allocated so far: " PROPERFMT ", limit: " PROPERFMT ") ", \
  NMTUtil::flag_to_enum_name(f), PROPERFMTARGS(s), PROPERFMTARGS(so_far), PROPERFMTARGS(limit->sz)

  if (limit->mode == MallocLimitMode::trigger_fatal) {
    fatal(FORMATTED);
  } else {
    log_warning(nmt)(FORMATTED);
  }

#undef FORMATTED

  return true;
}



then we can simply have:


// Returns true if allocating s bytes on f would trigger either global or the category limit
inline bool MallocMemorySummary::check_exceeds_limit(size_t s, MEMFLAGS f) {

  // Note: checks are ordered to have as little impact as possible on the standard code path,
  // when MallocLimit is unset, resp. it is set but we have reached no limit yet.
  // Somewhat expensive are:
  // - as_snapshot()->total(), total malloc load (requires iteration over arena types)
  // - VMError::is_error_reported() is a load from a volatile.
  if (MallocLimitHandler::have_limit()) {

    // Global Limit ?
    const malloclimit* l = MallocLimitHandler::global_limit();
    if (l->sz > 0) {
      size_t so_far = as_snapshot()->total();
      if ((so_far + s) > l->sz) { // hit the limit
        if (total_limit_reached(s, so_far, l)) {
          return true;
        }
      }
    } else {
      // Category Limit ?
      l = MallocLimitHandler::category_limit(f);
      if (l->sz > 0) {
        const MallocMemory* mm = as_snapshot()->by_type(f);
        size_t so_far = mm->malloc_size() + mm->arena_size();
        if ((so_far + s) > l->sz) {
          if (category_limit_reached(f, s, so_far, l)) {
            return true;
          }
        }
      }
    }
  }

  return false;
}



Doesn't that work?

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

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