Skip to content

Conversation

@roberttoyonaga
Copy link
Collaborator

@roberttoyonaga roberttoyonaga commented Nov 23, 2023

Summary

As a first step to supporting native memory tracking (NMT), this PR adds support for tracking mallocs/calloc/realloc. Virtual memory tracking support will be added in a separate PR as it is mostly independent of this code.

Related issue: #7631
This already includes the cleanup from: #7669

Implementation

This PR adopts a similar approach to NMT in Hotspot. It uses "malloc headers". A "malloc header" is used to cache metadata about the native allocation. To do this, a small amount of additional space is requested contiguous to the user allocation. Malloc headers are always the same size. This metadata is later used to update the memory tracking once the block is freed.
The usage of malloc headers requires certain precautions. Please see the javadoc on class NativeMemoryTracking for more info.

Alternative Approaches

One alternative approach to malloc headers is using a big look-up table to cache metadata. This approach has the advantage of being more robust. It does not have the disadvantages of malloc headers described in theNativeMemoryTracking javadoc.
However, it has a few big disadvantages of its own:

  1. Work would need to be done to tune the table size
  2. Access to the table would need to be synchronized. The benefit of malloc headers, is that no locking is required after initialization.

An alternative to enabling NMT at runtime is to decide whether to enable NMT at build-time. This would allow us to bypass a lot of complexity by eliminating the special handling done to accommodate a pre-init phase. This is a possibility not available in Hotspot. In this PR, I've decided to implement the handling required to postpone this decision until runtime, not because I firmly believe it's the best choice, but because it might help facilitate discussion on whether its worth the added flexibility. The code that handles pre-init is relatively independent and can be easily removed.
The advantages of enabling at runtime are:

  1. Parity with Hotspot
  2. More flexibility for the user.

The disadvantages are:

  1. Additional code complexity
  2. Allocations in the pre-init phase that live across the initialization boundary cannot be freed. This prevents their addresses from being reused. This may not be a big deal because the number of pre-init allocations that survive beyond initialization and would normally be freed shortly afterward is likely small.

Limitations

  • NMT has not been hooked up to JFR. Support for NMT JFR events are not yet added. This shouldn't be too hard to do, but I've left it out to avoid this PR getting too big. For now, a single snapshot of the data is logged at shutdown. This isn't very useful because you only get one datapoint.
  • Support is only for Linux. It should be possible to add Windows support later. Similar tweaks will be needed in the windows platform specific code.
  • Only a few native allocation call sites have been labelled (mtTest, mtNMT, mtTracing, mtThread, mtOther). Everything else is aggregated into the default mtNone for now. Manual labeling of other call sites can be done in future PRs.

@oracle-contributor-agreement
Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Nov 23, 2023
wip

handle preinit

clean up and add some manual labels

minor cleanup and fixes

Label call sites and clean up

mtThread

Remove virtual memory tracking code

wip

remove more vmem tracking code

comments and refactor

more small fixes, refactore, comments

comment

unchecked cast

label Unsafe callsite. Clean up. javadoc
@roberttoyonaga
Copy link
Collaborator Author

@fniephaus do you know why the OCA check is failing for me now?

@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Nov 24, 2023
@alina-yur
Copy link
Member

@roberttoyonaga sorry, there has been an issue on our side. It's now resolved, the check is passing. thank you for your contribution!


private final UninterruptibleEntry[] table;
private int size;
protected int size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this protected?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it protected so that com.oracle.svm.core.nmt.PreInitTable can decrement the size when it frees nodes using raw LibC https://github.com/oracle/graal/pull/7883/files#diff-87344ca5d4dbcfcc86a41d2c665f97dba67181f1a0ac4c0428b4a9799bc23b2dR77

I've added @Override and a brief javadoc on some of the methods here for more clarity.

@christianhaeubl christianhaeubl changed the title Add initial support for native memory tracking (NMT mallocs) [GR-51851] Add initial support for native memory tracking (NMT mallocs) Feb 5, 2024
@christianhaeubl
Copy link
Member

@roberttoyonaga : thanks, I started integrating this PR.

@roberttoyonaga
Copy link
Collaborator Author

@roberttoyonaga : thanks, I started integrating this PR.

Thank you @christianhaeubl. What do you think about allowing enabling NMT at runtime vs buildtime?

Also, I think it might be a good idea to pad the malloc header with 4B so it's a total of 16B for alignment reasons. I haven't done this yet, but should be easy and not have any side effects other than increase memory usage.

@christianhaeubl
Copy link
Member

I think that we should only have a hosted option for NMT (i.e., if the option is enable at image build-time, NMT is used at run-time).

As far as I can see, the malloc header is already 16 bytes (is rounded to wordSize).

@roberttoyonaga
Copy link
Collaborator Author

roberttoyonaga commented Feb 6, 2024

I think that we should only have a hosted option for NMT (i.e., if the option is enable at image build-time, NMT is used at run-time).

Ok, in that case, we can remove much of the code in NativeMemoryTracking.java

As far as I can see, the malloc header is already 16 bytes (is rounded to wordSize).

Right, I forgot about that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

native-image OCA Verified All contributors have signed the Oracle Contributor Agreement. redhat-interest

Projects

Status: Released

Development

Successfully merging this pull request may close these issues.

6 participants