RFR: 8297718: Make NMT free:ing protocol more granular [v2]

Gerard Ziemski gziemski at openjdk.org
Tue Dec 6 20:13:02 UTC 2022


On Thu, 1 Dec 2022 13:59:25 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> This PR avoids the double registration of a failed `os::realloc` in NMT by doing the NMT free protocol steps inline (asserting block integrity, marking of MallocHeader, and registrering results to statistics). This is done by creating a new struct `FreePackage` which includes all of the necessary data for registrering results.
>> 
>> I also did some light refactoring which I think makes the protocol clearer, these are done piece wise as separate commits for the reviewers' convenience.
>
> Johan Sjölen has updated the pull request incrementally with 10 additional commits since the last revision:
> 
>  - Add test that asserts block integrity after revival
>  - Make MallocHeader NONCOPYABLE
>  - Rest of it
>  - Adjust comments
>  - Put back comment, suitably rewritten
>  - Note reversibility of mark_block_as_dead
>  - Rename record_free_block to record_free
>  - Rename record_free to deaccount
>  - Move FreePackage into MallocHeader and rename
>  - Rename as revive, delete copy ctr

I had to dig into the NMT code to understand the change and this is my understanding:

**OLD:**

  MemTracker::record_free(memblock);
    MallocTracker::record_free(memblock);
      header->assert_block_integrity();
      MallocMemorySummary::record_free(header->size(), header->flags());
        as_snapshot()->by_type(flag)->record_free(size);
        as_snapshot()->_all_mallocs.deallocate(size);
      header->mark_block_as_dead();
        _canary = _header_canary_dead_mark;
         NOT_LP64(_alt_canary = _header_alt_canary_dead_mark);
         set_footer(_footer_canary_dead_mark);
  if FAILED(realloc):
    MemTracker::record_malloc(old_outer_ptr, old_size, memflags, stack);
      MallocTracker::record_malloc(mem_base, size, flag, stack);
        MallocMemorySummary::record_malloc(size, flags);
          as_snapshot()->by_type(flag)->record_malloc(size);
          as_snapshot()->_all_mallocs.allocate(size);
    return NULL


**NEW:**

  MallocHeader* const header = MallocTracker::malloc_header(memblock);
  header->assert_block_integrity();
  const MallocHeader::FreeInfo free_info = header->free_info();
  header->mark_block_as_dead();
    _canary = _header_canary_dead_mark;
    NOT_LP64(_alt_canary = _header_alt_canary_dead_mark);
    set_footer(_footer_canary_dead_mark);
  if FAILED(realloc):
    header->revive();
      _canary = _header_canary_life_mark;
      NOT_LP64(_alt_canary = _header_alt_canary_life_mark);
      set_footer(_footer_canary_life_mark);
    return NULL
  ELSE(realloc):
    MemTracker::deaccount(free_info);
      MallocMemorySummary::record_free(free_info.size, free_info.flags);
        as_snapshot()->by_type(flag)->record_free(size);
        as_snapshot()->_all_mallocs.deallocate(size);


So basically you are re-implementing `MemTracker::record_free()` without `MallocMemorySummary::record_free()`, which we will eventually call if `realloc()` succeeds.

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

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


More information about the hotspot-runtime-dev mailing list