Refactor log bits #1364
Refactor log bits #1364
Conversation
src/plan/plan_constraints.rs
Outdated
| @@ -45,6 +45,10 @@ pub struct PlanConstraints { | |||
| /// `MutatorConfig::prepare_func`). Those plans can set this to `false` so that the | |||
| /// `PrepareMutator` work packets will not be created at all. | |||
| pub needs_prepare_mutator: bool, | |||
| /// Should a policy unlog newly allocated objects? | |||
| pub unlog_allocated_object: bool, | |||
There was a problem hiding this comment.
I think this field needs to be defined per space, rather than per plan. In our generational plans, this value should be false for the nursery space and any space that allows mixed generation (such as LOS, or sticky immix's immix space). And the value should be true for other spaces.
There was a problem hiding this comment.
Hm. It should also be false for LOS since it has a nursery. What spaces do we directly allocate into that would need this for generational GCs? Immortal space, I guess. Because LOS and nursery space should be the main spaces where new objects are allocated into. Again, I'm only talking about generational GCs. I'm aware that SATB requires this.
Nvm. I misread what you said, sorry. Yes I agree with what you said.
There was a problem hiding this comment.
What spaces do we directly allocate into that would need this for generational GCs?
Immortal, non moving, code, VM space (we don't allocate into VM space, but we ask the binding to call our post_alloc after their allocation), etc. Also if we allow pretenure allocation, we may directly allocate into a mature space and also unlog the objects.
qinsoon
force-pushed
the
plan-use-log-bits
branch
from
017a060 to
62cb89e
Compare
This reverts commit 9c7695c.
| @@ -45,3 +45,6 @@ jobs: | |||
| cd mmtk-openjdk | |||
| export RUST_BACKTRACE=1 | |||
| ./.github/scripts/${{ inputs.test-script }} | |||
| - name: Setup tmate session | |||
There was a problem hiding this comment.
This was used for debugging. I will revert this.
|
@qinsoon Among the comments I have given above, only #1364 (comment) can be a result of potential bugs. Others are aesthetic comments which may be fixed or may be left as is, depending you your taste. The style check failure in the CI should be fixed by #1367 |
|
I think we need to ensure that the bug we found and fixed in #1169 is not re-introduced. |
|
There are new methods introduced in this PR that return |
qinsoon
force-pushed
the
plan-use-log-bits
branch
from
aed8dfe to
cabdd66
Compare
|
We saw a test failure in https://github.com/mmtk/mmtk-core/actions/runs/16955407376/job/48133843550?pr=1364. It is probably not related with this PR. I opened an issue for it: mmtk/mmtk-openjdk#317. |
This PR refactors the log bits: the current implementation assumes log bits are used for generational plans, and they carry generation semantics. This PR removes such assumptions.
With this PR, log bits are global metadata, and its semantic is defined by the plans. A plan may directly manipulate log bits, and may tell policies how to deal with log bits. A policy should never assume the semantics of log bits, and they just do whatever they are instructed to.
Some policies assume the log bit may be in the side, and this PR does not try to address this issue.