Skip to content

Remove the immix_stress_copying feature. Replace with separate options. - #1324

Merged
qinsoon merged 4 commits into
mmtk:masterfrom
qinsoon:refactor-immix-stress-defrag-features
May 21, 2025
Merged

Remove the immix_stress_copying feature. Replace with separate options.#1324
qinsoon merged 4 commits into
mmtk:masterfrom
qinsoon:refactor-immix-stress-defrag-features

Conversation

@qinsoon

@qinsoon qinsoon commented May 20, 2025

Copy link
Copy Markdown
Member

immix_stress_copying may cause confusion with the stress GC and the stress factor, as immix_stress_copying does not imply stress GCs. This PR removes immix_stress_copying, and replaces with separate options to control the specific behavior.

@qinsoon
qinsoon force-pushed the refactor-immix-stress-defrag-features branch from dbf67eb to a2682e4 Compare May 20, 2025 06:38
@qinsoon
qinsoon force-pushed the refactor-immix-stress-defrag-features branch from a2682e4 to 66eef3b Compare May 20, 2025 06:40
@qinsoon
qinsoon marked this pull request as ready for review May 20, 2025 07:23
@qinsoon
qinsoon requested a review from wks May 20, 2025 07:23
@qinsoon qinsoon changed the title Remove immix_stress_copying. Replace with immix_stress_defrag and immix_defrag_every_block Remove the immix_stress_copying feature. Replace with separate options. May 20, 2025
Comment on lines +6 to +7
One way to make copying more deterministic is to use the following options to
change the copying behavior of Immix.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the reader is new to MMTk, they may wonder how to set options. Providing a link to mmtk::util::options::Options should be helpful. We can also add some examples that set the options by setting environment variables.

Comment thread src/util/options.rs
Comment on lines +907 to +912
/// Mark every allocated block as defragmentation source before GC. (for debugging)
immix_defrag_every_block: bool [always_valid] = false,
/// Percentage of heap size reserved for defragmentation.
/// According to [this paper](https://doi.org/10.1145/1375581.1375586), Immix works well with
/// headroom between 1% to 3% of the heap size.
immix_defrag_headroom_percent: usize [|v: &usize| *v <= 50] = 2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Merely setting immix_defrag_every_block = true will make it run out of headroom before copying every live object. We should mention, in either immix_defrag_every_block or immix_defrag_headroom_percent or both, that we need to set immix_defrag_headroom_percent = 50 if we want to stress copying. We can also provide a link to the porting guide.

@qinsoon qinsoon May 21, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a good point. So setting immix_defrag_every_block to true alone does not make sense.

What if we set the defrag headroom to 50% but we do not set immix_defrag_every_block? I assume the defrag also works, as it will still defrag enough blocks (which should be all the blocks) to use the headroom.

I am wondering if we really need an option for immix_defrag_every_block.

@qinsoon

qinsoon commented May 21, 2025

Copy link
Copy Markdown
Member Author

I have made changes to address the reviews. Can you take a look again? @wks

@wks wks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

The mmtk-ruby binding currently has a feature named immix_stress_copying which is just a wrapper of the mmtk-core feature fo the same name. I'll remove it from mmtk-ruby once this PR is merged.

@qinsoon
qinsoon enabled auto-merge May 21, 2025 05:42
@qinsoon
qinsoon added this pull request to the merge queue May 21, 2025
Merged via the queue into mmtk:master with commit d538987 May 21, 2025
@qinsoon
qinsoon deleted the refactor-immix-stress-defrag-features branch May 21, 2025 07:34
wks added a commit to wks/mmtk-ruby that referenced this pull request May 21, 2025
The feature of the same name has been removed in mmtk-core.

See: mmtk/mmtk-core#1324
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants