Skip to content

Annotate mmap ranges using PR_SET_VMA #1236

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 24 commits into from
Nov 29, 2024
Merged

Annotate mmap ranges using PR_SET_VMA #1236

merged 24 commits into from
Nov 29, 2024

Conversation

wks
Copy link
Collaborator

@wks wks commented Nov 15, 2024

We demand that every invocation of mmap within mmtk-core to be accompanied with an "annotation" for the purpose of the mmap. On Linux, we will use PR_SET_VMA_ANON_NAME to set the attribute after mmap so that it can be seen in /proc/pid/maps. This will greatly improve the debugging experience.

We demand that every invocation of `mmap` within mmtk-core to be
accompanied with an "annotation" for the purpose of the mmap.  On Linux,
we will use `PR_SET_VMA` to set the annotation after `mmap` so that it
can be seen in `/proc/pid/maps`.  This will greatly improve the
debugging experience.
@k-sareen
Copy link
Collaborator

k-sareen commented Nov 16, 2024

Why not just use the space/metadata spec name instead? Also there's a 80-byte limit for the string (including the NUL byte).

However, for Map32, spaces may interleave at chunk granularity in the shared discontiguous memory range.

Why is this a problem? If a chunk is recycled, then we should just rename it.

@wks
Copy link
Collaborator Author

wks commented Nov 16, 2024

Why not just use the space/metadata spec name instead?

  • For space, I just use its name.
  • For side metadata, I use both space name and metadata spec name to distinguish between metadata for different spaces.
  • And there are mmap invocations for other purposes, too, such as SFTMap.

Also there's a 80-byte limit for the string (including the NUL byte).

Good point. Although I don't think any space name + metadata name would exceed this limit, it's safe to perform the check when formatting.

Why is this a problem? If a chunk is recycled, then we should just rename it.

The number of mmap entries is limited at the OS level. So whenever possible, we should merge adjacent entries. If no chunk in the discontiguous space has name (annotated with PR_SET_VMA_ANON_NAME), the OS may merge adjacent entries; but if adjacent entries have different names, they can't be merged.

@k-sareen
Copy link
Collaborator

k-sareen commented Nov 16, 2024

And there are mmap invocations for other purposes, too, such as SFTMap.

Yes that is a good point. Though in my branch I just use an Option<&str> because it's not always clear what name to give to an mmap region such as the side metadata implementation for 32-bits and for panic_if_unmapped.

The number of mmap entries is limited at the OS level. So whenever possible, we should merge adjacent entries. If no chunk in the discontiguous space has name (annotated with PR_SET_VMA_ANON_NAME), the OS may merge adjacent entries; but if adjacent entries have different names, they can't be merged.

I'm not sure this is a real concern? I don't imagine we'll ever be hitting the limit even for 32-bit machines. Even Android always has the annotations enabled. I strongly suggest having these on by default. If we ever hit the limit then we can disable it.

@wks wks marked this pull request as ready for review November 25, 2024 12:20
@wks wks added the PR-extended-testing Run extended tests for the pull request label Nov 25, 2024
@wks
Copy link
Collaborator Author

wks commented Nov 25, 2024

The number of mmap entries is limited at the OS level. So whenever possible, we should merge adjacent entries. If no chunk in the discontiguous space has name (annotated with PR_SET_VMA_ANON_NAME), the OS may merge adjacent entries; but if adjacent entries have different names, they can't be merged.

I'm not sure this is a real concern? I don't imagine we'll ever be hitting the limit even for 32-bit machines. Even Android always has the annotations enabled. I strongly suggest having these on by default. If we ever hit the limit then we can disable it.

Well, perhaps it's not a problem. I tried a few benchmarks. We do mmap at chunk granularity. This means we don't call mmap very often. It is proportional to the heap size, but even memory-hungry benchmarks like h2 run just fine, with only a few hundred invocations of mmap in the entire execution. And there are less than 20 entries in /proc/pid/mmap The following is captured while running the h2 benchmark on OpenJDK with 1GB heap size. They are the only mmaps created by mmtk-core (besides those created by malloc and pthread_create).

40000000-40400000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:nursery]
40400000-40800000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:los]
40800000-41800000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:nursery]
41800000-41c00000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:los]
41c00000-5f400000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:nursery]
5f400000-6f400000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:immix_mature]
6f400000-8f000000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:nursery]
8f000000-90800000 rw-p 00000000 00:00 0                                  [anon:mmtk:space:immix_mature]
100000000-1001e0000 rw-p 00000000 00:00 0 
1001e0000-140000000 ---p 00000000 00:00 0 
e0005000000-e0005800000 rw-p 00000000 00:00 0                            [anon:mmtk:sidemeta:nursery:VMGlobalLogBitSpec]
e0005800000-e0005c00000 rw-p 00000000 00:00 0                            [anon:mmtk:sidemeta:immix_mature:VMGlobalLogBitSpec]
e0005c00000-e0006400000 rw-p 00000000 00:00 0                            [anon:mmtk:sidemeta:nursery:VMGlobalLogBitSpec]
e0006400000-e0006800000 rw-p 00000000 00:00 0                            [anon:mmtk:sidemeta:immix_mature:VMGlobalLogBitSpec]
4e0800400000-4e0800c00000 rw-p 00000000 00:00 0                          [anon:mmtk:sidemeta:immix_mature:IX_LINE_MARK]
4e8800000000-4e8800400000 rw-p 00000000 00:00 0                          [anon:mmtk:sidemeta:immix_mature:IX_BLOCK_DEFRAG]
4e8900000000-4e8900400000 rw-p 00000000 00:00 0                          [anon:mmtk:sidemeta:immix_mature:IX_BLOCK_MARK]
4e8a00000000-4e8a00400000 rw-p 00000000 00:00 0                          [anon:mmtk:sidemeta:immix_mature:CHUNK_MARK]
4eaa82000000-4eaa82400000 rw-p 00000000 00:00 0                          [anon:mmtk:sidemeta:los:VMLocalLOSMarkNurserySpec]
4eac83000000-4eac84800000 rw-p 00000000 00:00 0                          [anon:mmtk:sidemeta:immix_mature:VMLocalMarkBitSpec]

So I think it's OK to leave it enabled by default. The user can still set MMTK_MMAP_ANNOTATION=false to disable it if it ever becomes problematic.

@k-sareen
Copy link
Collaborator

I think you need to update some of the tests to use the new annotation API as well.

Re: mmap chunks. Like I mentioned in a meeting, we can improve this further by allocating spaces from different ends of the address space

@k-sareen
Copy link
Collaborator

Err.. Why is sysinfo broken 🤔

@wks
Copy link
Collaborator Author

wks commented Nov 26, 2024

Err.. Why is sysinfo broken 🤔

There is an upstream bug report: GuillaumeGomez/sysinfo#1392

@k-sareen
Copy link
Collaborator

Should we pin version of sysinfo/libc until this is resolved then?

@wks
Copy link
Collaborator Author

wks commented Nov 26, 2024

Should we pin version of sysinfo/libc until this is resolved then?

I pinned the version of libc in this PR so that we can test and review this PR.

There is already a PR for the sysinfo crate that will fix this problem.

Copy link
Collaborator

@k-sareen k-sareen left a comment

Choose a reason for hiding this comment

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

LGTM except some minor stuff in review.

/// FIXME: Since it is set via `Options`, it is in theory a decision per MMTk instance. However, we
/// currently don't have a good design for multiple MMTk instances, so we use static variable for
/// now.
pub(crate) static MMAP_ANNOTATION: AtomicBool = AtomicBool::new(true);
Copy link
Member

Choose a reason for hiding this comment

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

This looks bad. Though there is a FIXME above, we should avoid introducing this at first place.

Both Plan and Space have references to the options. You can let plan/space to check options and create a MmapAnnotation. If the runtime option is not enabled, we can use Option<MmapAnnotation> or MmapAnnotation::Omit/None.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. That looks bad.

Although it is possible to let plans and spaces check options, and optionally pass MmapAnnotation::Omit or a None for Option<MmapAnnotation>, it is not ideal.

One important part of my intention is that the annotation cannot be omitted. I'd like the function signatures to enforce that an annotation must be provided for every call to mmap. If we give the caller the option to pass None or Omit, developers will do it if they think it is not worth annotating. Instead, the decision of whether to actually annotate the mmap should be postponed until mmap is actually called and is successful, and should be controlled by the user via an option rather than at call sites.

And checking at every call site is repetitive and error prone. Developers may forget to do the check or erroneously omit the annotation.

Ideally, functions related to memory mapping should be wrapped into a struct, maybe named MmapSupport or CommonMapper. It will be created per MMTK instance, like Mmapper and VMMap (which are currently both global singletons), and referenced by plans, spaces, SFT, etc., whatever needs to use mmap. The mmap_annotation: bool should be an instance variable of CommonMapper, and map_fixed should be an instance method of MmapSupport. That may involve much refactoring, including moving dzmmap, dzmmap_noreplace and mmap_noreserve into MmapSupport. For now, creating a global boolean variable seems to be the easiest solution.

In the short term, we may introduce MmapSupport as a global singleton, like Mmapper and VMMap. In the long term, we can make them specific to the MMTK instance.

Copy link
Member

Choose a reason for hiding this comment

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

One important part of my intention is that the annotation cannot be omitted. I'd like the function signatures to enforce that an annotation must be provided for every call to mmap.

We could enforce the users to supply MmapAnnotation, and turn it into Option<MmapAnnotation> before we send it to mmap functions. Plus the problem of having a global var outweighs the benefits of 'enforcing annotations'.

Alternatively, we can have an instance for memory which holds the boolean value from the option. I think they are both worth trying before adopting the current implementation. We have arg structs for creating plans and spaces. Creating a Memory instance and letting plan/space to hold it shouldn't be too hard.

pub struct CreateGeneralPlanArgs<'a, VM: VMBinding> {

It will be created per MMTK instance, like Mmapper and VMMap (which are currently both global singletons), and referenced by plans, spaces, SFT, etc., whatever needs to use mmap.

I think it is still arguable whether Mmapper and VMMap should be global or per instance. We do have a hacky vm_layout() that should be per instance, and is global at the moment. But we should avoid introducing more globals like this. The more we have, the more difficult and the more resistance we will face when we try to support multiple instances.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alternatively, we can have an instance for memory which holds the boolean value from the option. I think they are both worth trying before adopting the current implementation. We have arg structs for creating plans and spaces. Creating a Memory instance and letting plan/space to hold it shouldn't be too hard.

pub struct CreateGeneralPlanArgs<'a, VM: VMBinding> {

Yes. This is my intended way to do it. But this may involve too many changes and I think it is better to do the refactoring separately.

The introduced instance (let's call it MmapSupport for now) should contain dzmap, dzmap_noreplace, mmap_noreserve and mmap_fixed. I grepped the code and found that the following places will be affected.

  • MapState: MapState::transition_to_* will call those functions directly, as if the MapState can transition its own state without the help of Mmapper or the MmapSupport instance. This needs to be refactored so that MapState will become a state holder, and Mmapper implementations (FragmentedMapper and ByteMapMmapper) can call the mmap functions in MmapSupport to do the transition.
  • LockFreeImmortalSpace: It skipped the Mmapper and directly calls dzmmap_noreplace. I think this is a bug because Mmapper should be aware of the mapping of spaces.
  • RawMemoryFreeList: It calls dzmmap_noreplace to map its memory. Not sure if it should go through Mmapper, but probably it should because a RawMemoryFreeList is part of a space using it.

In the end, we may have all mmap invocations going through Mmapper, and an Mmapper holds a reference to MmapSupport.

We could enforce the users to supply MmapAnnotation, and turn it into Option<MmapAnnotation> before we send it to mmap functions.

It is good to enforce the users to supply MmapAnnotation. The difficulty is drawing a border between the "user" and the "mmap functions". And another difficulty is that we lost access to Options before reaching "mmap functions".

Take the call chain SideMetadataContext::map_metadata_internal -> try_mmap_contiguous_metadata_space -> MMAPPER.ensure_mapped for example.

  • In SideMetadataContext::map_metadata_internal, we have enough information (space name and side metadata name) to construct the annotation. We construct MmapAnnotation here. But since SideMetadataContext does not reference anySpace or Plan, it cannot access Options.
  • try_mmap_contiguous_metadata_space is a top-level function. It has access to neither space name nor Option.
  • MMAPER.ensure_mapped is a function that is more related to mmap. It may be refactored to hold a boolean variable MMAP_ANNOTATION, or refer to MmapSupport I mentioned above.

So the availability of Options or the MMAP_ANNOTATION variable does not match the distinction between user and mmap. I think it's easier to refactor the code and just let "mmap functions" decide whether to annotate.

Plus the problem of having a global var outweighs the benefits of 'enforcing annotations'.

Having global var is bad, but I think it is worse to let the users check Options::mmap_annotation before constructing the Option<MmapAnnotation> because it is repetitive and error-prone. Having a global variable is the easiest workaround before we refactor Mmapper, MapState, etc.

If you think having a global variable is unacceptible, I can remove Options::mmap_annotation and make the annotation controlled by a Cargo feature instead, or leave it always on. I think it is OK to leave it always on because there will be strictly fewer prctl than mmap, and we don't call mmap very often, either.

Copy link
Member

Choose a reason for hiding this comment

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

If you think having a global variable is unacceptible, I can remove Options::mmap_annotation and make the annotation controlled by a Cargo feature instead, or leave it always on.

Right. We dont really need the option anyway. We can always annotate the mmaps.

The reason that I am against having a global var is that we know it's wrong and this kind of small issues will accumulate over time and eventually make it hard to get rid of them. We should try to avoid introducing them in first place.

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 removed the option, and added a Cargo feature no_mmap_annotation just in case anyone needs to disable it for any reason.

Copy link
Member

Choose a reason for hiding this comment

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

This global var can be removed now. I am suprised that we did not get any warning from the compiler for this, this seems to be an unused variable.

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

Generally looks good. The global var MMAP_ANNOTATION can be removed.

"failed to mmap meta memory"
);
CHUNK_METADATA
.try_map_metadata_space(start, size, space_name)
Copy link
Member

Choose a reason for hiding this comment

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

This is good refactoring with clear improvement. The old code put the actual action performed in the assert! which is bad.

/// FIXME: Since it is set via `Options`, it is in theory a decision per MMTk instance. However, we
/// currently don't have a good design for multiple MMTk instances, so we use static variable for
/// now.
pub(crate) static MMAP_ANNOTATION: AtomicBool = AtomicBool::new(true);
Copy link
Member

Choose a reason for hiding this comment

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

This global var can be removed now. I am suprised that we did not get any warning from the compiler for this, this seems to be an unused variable.

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

Looks good

@wks
Copy link
Collaborator Author

wks commented Nov 29, 2024

V8 failed for HTTP 404. I think it might be a transient failure if the Ubuntu server is updating itself and happens to be in an inconsistent state when we ran the test. See: https://github.com/mmtk/mmtk-core/actions/runs/12078239098/job/33686682978?pr=1236

JikesRVM failed for the classic "instruction ... not in RVM space" error. Interestingly, the dumped maps file showed that our annotation is working. See: https://github.com/mmtk/mmtk-core/actions/runs/12078239098/job/33686683158?pr=1236

I'll rerun those tests.

@wks wks added this pull request to the merge queue Nov 29, 2024
Merged via the queue into mmtk:master with commit cd2fe83 Nov 29, 2024
29 of 34 checks passed
@wks wks deleted the feature/mmap-anno branch November 29, 2024 10:18
github-merge-queue bot pushed a commit that referenced this pull request Dec 2, 2024
#1242 fixed most similar issues in
the repo, but #1236 introduced
`MmapAnnotation` and introduced a new warning.
qinsoon added a commit to qinsoon/mmtk-core that referenced this pull request Mar 25, 2025
commit e787b2d
Merge: d56c3b9 0b3ec9b
Author: Yi Lin <qinsoon@gmail.com>
Date:   Tue Mar 25 03:02:42 2025 +0000

    Merge branch 'master' into check-fragmentation-immixspace

commit 0b3ec9b
Author: Kunal Sareen <kunal.sareen@anu.edu.au>
Date:   Mon Mar 24 17:11:27 2025 +1100

    Make work packet buffer size configurable from one location (mmtk#1285)

    Closes mmtk#1281

commit 129362d
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Thu Mar 20 17:16:35 2025 +0800

    Fix lychee command (mmtk#1286)

    Removed `--base`. All relative URLs in Markdown are relative to the file
    itself.

    Increased verbosity to print the checked URLs and redirections.

    Excluded `dl.acm.org`. It always responses 403 when using Lychee to
    check links.

    ---------

    Co-authored-by: Yi Lin <qinsoon@gmail.com>

commit c4f5a02
Author: Yi Lin <qinsoon@gmail.com>
Date:   Thu Mar 20 16:58:33 2025 +1300

    Fix to bytemuck_derive 1.8.1 (mmtk#1288)

    `bytemuck` uses `bytemuck_derive ^1.4.1`. Recent `bytemuck_derive`
    versions are not compatible with our MSRV 1.74.1 (1.9.1 requires Rust
    1.84+, 1.9.0 requires edition 2024 which is stablized in 1.84). So fix
    `bytemuck_derive` to the last version before 1.9.0.

commit 1a78557
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Mon Mar 17 17:37:41 2025 +0800

    Fixing MSRV-breaking dependencies (mmtk#1284)

    Some dependencies started to require MSRV above our current MSRV.

    The build dependency `built` transitively depends on some crates from
    the ICU4X project that recently started to depend on Rust 1.81, such as
    `litemap`. The dependency on ICU4X is completely unnecessary, and we
    removed it from our dependency tree by forcing the use of a particular
    version of `idna_adapter`. See: https://docs.rs/crate/idna_adapter/1.2.0

    The dev dependency `criterion` depends on `ciborium` which depends on
    the `half` crate. Since v2.5.0, `half` started to depend on Rust 1.81.
    `ciborium` needs to be fixed because its MSRV is 1.58 and shouldn't
    depend on a crate that requires Rust 1.81. We lock the version of the
    `half` to 2.4.1. This kind of problem should be properly addressed with
    the new MSRV-aware dependency resolver introduced in Rust 1.84. See:
    https://doc.rust-lang.org/cargo/reference/resolver.html#rust-version

commit 8dded8f
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Wed Feb 26 14:51:51 2025 +0800

    Fix clippy warning about operator precedence (mmtk#1280)

    In Rust 1.85, Clippy started to warn about the precedence of `<<` and
    `|` in one of our use cases, although that lint was added before Rust
    1.29.

    https://rust-lang.github.io/rust-clippy/master/index.html#precedence

commit 3832b0d
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Wed Feb 26 13:38:49 2025 +0800

    Remove dead trace_object methods. (mmtk#1277)

    Now that we are using the `#[derive(HasSpaces, PlanTraceObject)]` derive
    macros to generate the trace_object methods, we can remove the
    manually-written dead code.

commit 84545cc
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Thu Feb 20 16:19:48 2025 +0800

    Special topic chapter for finalizers and weak references (mmtk#1265)

    This PR adds a special topic chapter in the Porting Guide for supporting
    finalizers and weak references. This topic is frequently asked and
    somewhat complex, and needs a dedicated chapter.

    We also updated the doc comments of the `Scanning::process_weak_refs`
    API to add code example of the intended use case, and warn the users
    about potential pitfalls.

commit df5e0cd
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Thu Feb 20 11:27:05 2025 +0800

    Bump MSRV to 1.74.1 (mmtk#1276)

    The current latest version of the "built" crate (v0.7.7) requires MSRV
    1.74. We bump the MSRV to 1.74.1.

    Since version 0.7.6 of the "built" crate, it generates `static` items
    instead of `const` items for `PKG_VERSION`, `FEATURES_STR`, etc. Our
    `build_info.rs` used to define `const` items that take their values.
    After this change, the Rust compiler now interpret those lines as taking
    references of `static` items, which is unstable until Rust 1.83. We
    instead replaced those `const` items in `build_info.rs` with `use`
    statements that create aliases of the items generated by "built".

    Bumping MSRV to 1.74.1 also allows us to bump the version of the
    dependency "criterion" to 0.5 which also requires MSRV 1.74. Previously,
    we locked the version of "criterion" to 0.4 due to its MSRV requirement.

    We also updated all dependencies to their latest versions. Among those
    changes, the "sysinfo" crate renamed several `new` methods to `nothing`.
    We make changes accordingly.

    We also use `usize::div_ceil` which was introduced in Rust 1.73. This
    fixes a clippy warning.

commit 4ca8812
Author: Yi Lin <qinsoon@gmail.com>
Date:   Thu Feb 6 12:30:42 2025 +1300

    Fix julia extended tests (mmtk#1270)

    This PR changes the extended testing workflow for Julia:
    1. It now tests with `master` in the upstream Julia repo, and `master`
    in the binding repo by default.
    2. In addition to specifying the binding version, we can also specify
    the Julia version to run with. This change is necessary, as we no longe
    record the Julia version in the binding (we record the binding version
    in Julia instead).

commit 054feef
Author: tianleq <90177881+tianleq@users.noreply.github.com>
Date:   Wed Jan 29 14:07:49 2025 +1100

    Clear stale line mark state (mmtk#1268)

    ![image](https://github.com/user-attachments/assets/5ce787c6-03c0-4845-b0a9-ade68bb0f8d6)

    ![image](https://github.com/user-attachments/assets/605711ba-1140-42b0-9306-849d4e15e244)

    ![image](https://github.com/user-attachments/assets/2f1ba1d5-9ca8-4678-b75a-45eac5789990)
    Overall, this fix does not incur significant overhead

commit 051bc74
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Tue Jan 21 13:31:06 2025 +0800

    Make GC triggering and heap resizing consistent (mmtk#1266)

    This PR fixes a bug where the MemBalancer did not increase the heap size
    enough to accommodate the amount of side metadata needed by the pending
    allocation. It manifested as looping infinitely between triggering GC
    and (not actually) resizing the heap size after a GC when the minimum
    heap size is too small.

    Now it always includes the side metadata amount when increasing heap
    size.

    This PR also refactors the calculation of "shifting right and rounding
    up" which is used in multiple places. We also replace `alloc_rshift`
    with `log_data_meta_ratio` for two reasons. (1) The previous
    implementation would cause unsigned overflow before converting the
    result to `i32`. (2) `log_data_meta_ratio` has clearer semantics.

commit c61e6c8
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Thu Jan 16 20:05:08 2025 +0800

    Force fixed heap size when using NoGC (mmtk#1264)

    The dynamic heap size trigger needs GC to adjust the heap size, but NoGC
    can't do GC. So it doesn't make sense to use dynamic heap size with
    NoGC. Currently, if the user selects the NoGC plan and the dynamic heap
    size trigger, it will trigger GC at the minimum heap size and then panic
    immediately.

    With this change, MMTk will give a warning and use fixed heap size
    trigger instead, using the maximum heap size specified in the dynamic
    trigger as the heap size.

commit 2f6f078
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Tue Jan 14 21:03:49 2025 +0800

    Fix Clippy warning in 1.84.0 (mmtk#1262)

    Rust 1.84.0 added a new lint "unnecessary_map_or". We use
    `Option::is_some_and` (introduced in Rust 1.70.0) as suggested by the
    lint.

commit 541298f
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Tue Jan 14 21:03:46 2025 +0800

    Fix a subtraction overflow in get_free_pages. (mmtk#1261)

    The used pages can also be greater than the total pages for the same
    reason as those in computing `get_available_pages`, and it can also
    happen if the VM binding disabled GC, in which case we may over-allocate
    without triggering GC. When it overflows, `get_free_pages` will cause
    subtraction overflow, and will panic in debug build.

    We switch to `saturating_sub` so that it will return 0 if overflow
    happens. It still makes sense. 0 means there is no free pages because we
    are over-allocating beyond the current heap size set by the GC trigger.

commit 68bf1b6
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Thu Jan 9 14:40:42 2025 +0800

    Unique object enqueuing option (mmtk#1255)

    Added a constant `VMBinding::UNIQUE_OBJECT_ENQUEUING`. When set to true,
    MMTk will guarantee that each object is enqueued at most once in each
    GC. This can be useful for VMs that piggyback on object scanning to
    visit objects during GC.

    Implementation-wise, the mark bit is set atomically when
    `VMBinding::UNIQUE_OBJECT_ENQUEUING` is true. This PR only affects the
    native MarkSweep space. Other spaces already do this atomically.

    Fixes: mmtk#1254

commit ec74535
Author: Yi Lin <qinsoon@gmail.com>
Date:   Tue Jan 7 09:56:50 2025 +1300

    Move to Rust 1.83 (mmtk#1253)

    This PR updates our pinned Rust version to 1.83. This also updates
    `ci-perf-kit` https://github.com/mmtk/ci-perf-kit/releases/tag/0.8.2
    that includes this Rust 1.83 migration as a new epoch.

commit c0f9788
Author: Yi Lin <qinsoon@gmail.com>
Date:   Fri Dec 20 15:28:58 2024 +1300

    Bump version to v0.30 (mmtk#1252)

commit 2e548e5
Author: Yi Lin <qinsoon@gmail.com>
Date:   Mon Dec 9 21:23:21 2024 +1300

    Allow setting object metadata for VM space objects. Expose VO bit under a feature. (mmtk#1248)

    This PR changes a few things for vo bit:
    1. Add a function to set object metadata for an object in the VM space:
    `MMTK::initialize_vm_space_object`.
    2. Add a feature `vo_bit_access` to expose VO bit and a binding may use
    it at its own risk.
    3. Mark VO bit side metadata base address only avilable for 64 bits.

    The second is needed for Julia. The Julia binding uses MMTk immortal
    allocation or VM space for a region of memory, and pop the regions with
    boot image objects with no clear way to identify each object. The easist
    workaround is to bulk set VO bit for the region. The problem from this
    is that MMTk cannot identify valid objects in those regions. However,
    Julia binding only uses VO bit for pinning objects. Objects in the
    immortal space or the VM space will not be moved so failing to pinning
    objects in those regions is benign. Currently the Julia binding
    duplicates a bunch of side metadata code to bulk set VO bit only using
    the VO bit side metadata base address. See
    mmtk/mmtk-julia#200

commit 3c1418a
Author: Yi Lin <qinsoon@gmail.com>
Date:   Mon Dec 9 18:26:42 2024 +1300

    Check the option before aggregating live bytes data. Panic if the option is enabled on malloc space. (mmtk#1250)

    We see failures
    [here](https://github.com/mmtk/mmtk-core/actions/runs/12193674159/job/34020377988?pr=1248)
    in OpenJDK tests.

    ```
    [2024-12-06T07:09:55Z INFO  mmtk::memory_manager] Initialized MMTk with MarkSweep (FixedHeapSize(54525952))
    [2024-12-06T07:09:55Z WARN  mmtk::memory_manager] The feature 'extreme_assertions' is enabled. MMTk will run expensive run-time checks. Slow performance should be expected.
    ===== DaCapo fop starting =====
    [2024-12-06T07:10:07Z INFO  mmtk::util::heap::gc_trigger] [POLL] MallocSpace: Triggering collection (13313/13312 pages)
    thread '<unnamed>' panicked at /home/runner/work/mmtk-core/mmtk-core/mmtk-openjdk/repos/mmtk-core/src/policy/marksweepspace/malloc_ms/global.rs:158:9:
    internal error: entered unreachable code
    stack backtrace:
       0: rust_begin_unwind
                 at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/panicking.rs:647:5
       1: core::panicking::panic_fmt
                 at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:72:14
       2: core::panicking::panic
                 at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:144:5
       3: <mmtk::policy::marksweepspace::malloc_ms::global::MallocSpace<VM> as mmtk::policy::space::Space<VM>>::common
                 at ./repos/mmtk-core/src/policy/marksweepspace/malloc_ms/global.rs:158:9
       4: mmtk::policy::space::Space::get_descriptor
                 at ./repos/mmtk-core/src/policy/space.rs:332:9
       5: mmtk::mmtk::MMTK<VM>::aggregate_live_bytes_in_last_gc::{{closure}}
                 at ./repos/mmtk-core/src/mmtk.rs:542:29
       6: <mmtk::plan::marksweep::global::MarkSweep<VM> as mmtk::plan::global::HasSpaces>::for_each_space
                 at ./repos/mmtk-core/src/plan/marksweep/global.rs:31:10
       7: mmtk::mmtk::MMTK<VM>::aggregate_live_bytes_in_last_gc
                 at ./repos/mmtk-core/src/mmtk.rs:540:9
       8: <mmtk::scheduler::gc_work::Release<C> as mmtk::scheduler::work::GCWork<<C as mmtk::scheduler::work::GCWorkContext>::VM>>::do_work
                 at ./repos/mmtk-core/src/scheduler/gc_work.rs:162:13
       9: mmtk::scheduler::work::GCWork::do_work_with_stat
                 at ./repos/mmtk-core/src/scheduler/work.rs:45:9
      10: mmtk::scheduler::worker::GCWorker<VM>::run
                 at ./repos/mmtk-core/src/scheduler/worker.rs:255:13
      11: mmtk::memory_manager::start_worker
                 at ./repos/mmtk-core/src/memory_manager.rs:491:5
      12: start_worker
                 at ./mmtk/src/api.rs:214:9
      13: _ZN6Thread8call_runEv
                 at ./repos/openjdk/src/hotspot/share/runtime/thread.cpp:402:12
      14: thread_native_entry
                 at ./repos/openjdk/src/hotspot/os/linux/os_linux.cpp:826:19
      15: <unknown>
      16: <unknown>
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
    fatal runtime error: failed to initiate panic, error 5
    ```

    The issue is that `aggregate_live_bytes_in_last_gc` was not guarded by
    the condition that the option `count_live_bytes_in_gc` is enabled. So it
    was executed in our tests.

    The function accesses the space descriptor through `CommonSpace` and
    `MallocSpace` does not use `CommonSpace`, thus we see the panic. This PR
    adds a check before calling `aggregate_live_bytes_in_last_gc`. When the
    option is not enabled, we will not call the function.

    This PR also adds a panic for `MallocSpace`. If `count_live_bytes` is
    turned on, we simply panic, as we cannot provide live bytes vs total
    page stats for `MallocSpace`.

commit e8ff7c6
Author: Yi Lin <qinsoon@gmail.com>
Date:   Thu Dec 5 15:32:12 2024 +1300

    Use macos-15 for style check (mmtk#1249)

    mmtk#1216 updated the test runner
    image from `macos-12` to `macos-15`, but I forgot to update the image
    for style checks. This PR updates the runner for style checks as well.

commit a753093
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Tue Dec 3 17:37:46 2024 +0800

    Minor changes for debugging. (mmtk#1245)

    Added `MMTK::debug_print_vm_map` which prints the memory ranges of
    spaces.

    `NullableObjectReference` now implements `Clone`, `Copy`, `Display` and
    `Debug`. This allows the binding to print its value like `Address` and
    `ObjectReference`, and is useful for logging API functions that involve
    `NullableObjectReference` parameters.

commit 8a398e0
Author: Yi Lin <qinsoon@gmail.com>
Date:   Tue Dec 3 17:03:34 2024 +1300

    Collect live bytes per space, and report by space (mmtk#1238)

    The current `count_live_bytes_in_gc` feature adds the size of all live
    objects and compare with the used pages reported by the plan. There are
    two issues with the feature:
    1. VM space is not included in the used pages reported by the plan, but
    the live objects include objects in the VM space. So the reported
    fragmentation/utilization is wrong when the VM space is in use.
    2. Spaces/policies have very different fragmentation ratio. Reporting
    the fragmentation for the entire heap is not useful.

    This PR refactors the current `count_live_bytes_in_gc` feature so we
    collect live bytes per space, and report by space.

commit 3d7bc11
Author: Yi Lin <qinsoon@gmail.com>
Date:   Mon Dec 2 14:55:37 2024 +1300

    Fix warnings for lifetime in MmapAnnotation impl (mmtk#1244)

    mmtk#1242 fixed most similar issues in
    the repo, but mmtk#1236 introduced
    `MmapAnnotation` and introduced a new warning.

commit cd2fe83
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Fri Nov 29 17:17:14 2024 +0800

    Annotate mmap ranges using PR_SET_VMA (mmtk#1236)

    We demand that every invocation of `mmap` within mmtk-core to be
    accompanied with an "annotation" for the purpose of the mmap. On Linux,
    we will use `PR_SET_VMA_ANON_NAME` to set the attribute after `mmap` so
    that it can be seen in `/proc/pid/maps`. This will greatly improve the
    debugging experience.

commit 5bc6ce5
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Fri Nov 29 13:16:22 2024 +0800

    Fix clippy warnings for Rust 1.83 (mmtk#1242)

    Clippy 1.83 produces some new warnings:

    - `needless_lifetimes` is extended to suggest eliding `impl` lifetimes.
    -   `empty_line_after_doc_comments` is added to the `suspicious` group.

commit 8640ab8
Author: Yi Lin <qinsoon@gmail.com>
Date:   Fri Nov 8 19:36:42 2024 +1300

    Bump version to v0.29 (mmtk#1232)

commit 41501a5
Author: Kunal Sareen <kunal.sareen@anu.edu.au>
Date:   Thu Nov 7 13:35:00 2024 +1100

    Fix nightly build and add `inline` attributes to `{un,}likely` (mmtk#1228)

commit 753f71c
Author: Yi Lin <qinsoon@gmail.com>
Date:   Thu Nov 7 15:17:58 2024 +1300

    Fix auto merge branches (mmtk#1230)

    This PR changes the auto merge workflow. For each binding, the workflow
    now allows inputs for base repo and base ref. This change is mostly for
    the Julia binding which uses `dev` instead of `master` as the default
    branch. mmtk#1221 only changed for the
    correctness testing, this PR made corresponding changes for auto merge.

commit 59ea62e
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Thu Nov 7 05:31:13 2024 +0800

    Use modern syntax for optional dependencies (mmtk#1229)

    Use the "dep:" prefix to specify optional dependencies in Cargo
    features.

    An optional crate dependency implicitly generates a feature of the same
    name, and can be leaked to the user of the current crate. But if a
    feature specifies a crate dependency with the "dep:" prefix, it will not
    implicitly generate the feature, hiding it from the users. The "dep:"
    prefix was introduced in Rust 1.60.

commit 3575521
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Wed Nov 6 13:49:45 2024 +0800

    Make env_logger an optional dependency (mmtk#1226)

    Now the built-in `env_logger` is guarded behind a Cargo feature
    "builtin_env_logger". It is a default feature, but can be disabled in
    Cargo.toml by setting `dependencies.mmtk.default-features = false`. In
    this way, VM bindings that want to implement its own logger can remove
    the `env_logger` crate from its dependencies.

    Fixes: mmtk#744

commit 3830168
Author: Yi Lin <qinsoon@gmail.com>
Date:   Fri Nov 1 15:02:03 2024 +1300

    Change the default testing branch for Julia tests (mmtk#1221)

    We recently re-organised branches in `mmtk-julia` and `julia`. Namely
    the previous `master` was renamed to `dev`, and we will use `master` for
    the version that works with Julia upstream. This PR extracts the default
    testing repos and branches, and changes the default testing branch for
    Julia.

commit 618fde4
Author: Yi Lin <qinsoon@gmail.com>
Date:   Mon Oct 21 18:48:08 2024 +1300

    Document the policy about performance testing environment and epochs (mmtk#1206)

    Co-authored-by: Kunshan Wang <wks1986@gmail.com>

commit f032697
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Mon Oct 21 13:46:06 2024 +0800

    Performance history canary (mmtk#1209)

    This PR adds a "canary" build to the performance regression CI of
    OpenJDK. The "canary" is a chosen revision of mmtk-core and mmtk-openjdk
    that is tested alongside each merged PR. The performance of the "canary"
    should not change unless there is an environment change or there is a
    noise. Spotting a change in the "canary" performance can help us
    identify environment changes that are unintended or otherwise unnoticed,
    and also identify the noise level.

    Currently, we choose a specific release version as the version of the
    "canary". Using a release version has the advantage of being easy to
    specify the exact revision of both the mmtk-core and the mmtk-openjdk
    repository. We may also switch to some methods of automatically select
    the canary version in the future.

    There are other minor changes made.

    - We slightly change the directory structure. We create two directory,
    namely `latest` and `canary`. In each of the directories, we check out
    `mmtk-core` and `mmtk-openjdk` of the latest and the canary versions,
    respectively.
    - We use the `ci-replace-mmtk-dep.py` script to replace the revision of
    the `mmtk-core` dependency in `mmtk-openjdk`. As a result, we no longer
    need to use `sed`, and no longer need to copy the `mmtk-core` directory
    into `mmtk-openjdk/repos`.
    -   We no longer set the `RUSTUP_TOOLCHAIN` environment variable because
    1. The latest and the canary version may not use the same toolchain,
    and,
    2. the right toolchain will be chosen when running the `cargo` command
    according to the `rust-toolchain` file in the directory.
    - The scripts in https://github.com/mmtk/ci-perf-kit are changed to take
    the canary into consideration, too.

commit 80b11a0
Author: Patrick LaFontaine <32135464+Pat-Lafon@users.noreply.github.com>
Date:   Sun Oct 20 20:05:29 2024 -0400

    Remove space for nogc link (mmtk#1217)

commit 0883898
Author: Yi Lin <qinsoon@gmail.com>
Date:   Thu Oct 17 20:00:03 2024 +1300

    Update CI macos image (mmtk#1216)

    macos-12 will no longer be supported:
    actions/runner-images#10721

commit 328deb6
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Fri Oct 11 16:07:53 2024 +0800

    Fix a race between forwarding bits and VO bits. (mmtk#1214)

    The current code sets the forwarding bits before setting the VO bit when
    copying an object. If another GC worker is attempting to forward the
    same object, it may observe the forwarding bits being `FORWARDED` but
    the VO bit is not set. This violates the semantics of VO bits because VO
    bits should be set for both from-space and to-space copies. This will
    affect VM bindings that assert slots always refer to a valid object when
    scanning objects and may update the same slot multiple times for some
    reasons.

    This revision provides a mechanism to ensure that all necessary metadata
    are set before setting forwarding bits to `FORWARDED`. Currently it
    affects the VO bits and the mark bits (which are used to update the VO
    bits in Immix-based plans). It may be used for other metadata introduced
    in the future.

commit 58b3b35
Author: Kunshan Wang <wks1986@gmail.com>
Date:   Fri Oct 11 14:04:55 2024 +0800

    Install cargo-msrv using stable toolchain. (mmtk#1215)

    When running the CI check "msrv", we install the cargo-msrv command
    using the stable Rust toolchain because it sometimes requires a higher
    Rust version than our chosen version in the file `rust-toolchain`.

commit c4fdce0
Author: Yi Lin <qinsoon@gmail.com>
Date:   Fri Sep 27 22:27:47 2024 +1200

    Bump version to v0.28 (mmtk#1212)

    Merge this PR after mmtk#1208 and
    mmtk#1211.

commit de10fa4
Author: Yi Lin <qinsoon@gmail.com>
Date:   Fri Sep 27 16:41:37 2024 +1200

    Update migration guide for mmtk#1205 (mmtk#1211)

    Co-authored-by: Kunshan Wang <wks1986@gmail.com>

commit 7cfebda
Author: Yi Lin <qinsoon@gmail.com>
Date:   Fri Sep 27 15:37:20 2024 +1200

    Update ci-perf-kit to plot epoch (mmtk#1208)

    Use mmtk/ci-perf-kit#46 to plot performance
    data.

commit 5605237
Author: Kunal Sareen <kunal.sareen@anu.edu.au>
Date:   Fri Sep 20 16:37:13 2024 +1000

    Return if a GC ran or not for `handle_user_collection_request` (mmtk#1205)

    Closes mmtk#1204

    ---------

    Co-authored-by: Yi Lin <qinsoon@gmail.com>

commit d56c3b9
Merge: 4cfac97 dd84218
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Wed Mar 27 00:26:59 2024 +0000

    Merge remote-tracking branch 'mmtk/master' into feature/check-fragmentation-immixspace

commit 4cfac97
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Tue Mar 26 23:27:59 2024 +0000

    Refactor code; turn on logs; set block size to 16K

commit aeb3aeb
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Wed Mar 6 23:12:08 2024 +0000

    Adding statistics about number of objects scanned and objects moved in immixspace

commit 6f1c924
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Wed Mar 6 23:02:10 2024 +0000

    Change printing info

commit f294948
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Tue Mar 5 01:08:04 2024 +0000

    Adding assertion for live lines in immixspace; properly counting live bytes in los

commit 184822c
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Tue Mar 5 01:07:19 2024 +0000

    Removing dependency on chrono

commit 1547428
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Mon Mar 4 09:52:28 2024 +0000

    Print stats for sticky immix as well

commit 06284e1
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Mon Mar 4 09:37:45 2024 +0000

    Adding feature to dump memory stats (from los and immixspace)

commit 1f39198
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Mon Mar 4 04:12:19 2024 +0000

    Trying to count live blocks and live lines

commit bd3305f
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Mon Mar 4 03:26:42 2024 +0000

    Zeroing the live bytes right before GC starts

commit ee21a9c
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Thu Feb 29 22:59:56 2024 +0000

    Moving stats from global state to immixspace

commit 530051b
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Thu Feb 29 04:33:27 2024 +0000

    Refactor range check

commit f081e4f
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Thu Feb 29 04:23:19 2024 +0000

    Removing duplicated method

commit c2a79ad
Author: Eduardo Souza <ledusou@gmail.com>
Date:   Thu Feb 29 04:13:55 2024 +0000

    Adding feature to query the fragmentation of immixspace
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
PR-extended-testing Run extended tests for the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants