Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove policy-specific code from plans, create space-specific work packets #258

Closed
steveblackburn opened this issue Feb 24, 2021 · 2 comments · Fixed by #575
Closed

Remove policy-specific code from plans, create space-specific work packets #258

steveblackburn opened this issue Feb 24, 2021 · 2 comments · Fixed by #575
Labels
A-work-queue Area: Work queue C-cleanup Category: Cleanup C-enhancement Category: Enhancement C-refactoring Category: Refactoring
Milestone

Comments

@steveblackburn
Copy link
Contributor

Policy-specific code should not be in the Plans. The old MMTk use the plans only for dispatch to policies (eg a switch statement dispatching to various implementations of traceObject().

For both software engineering and performance reasons, work packets should be homogenous w.r.t. the type of work they do.

So for something like traceObject(), there should be a separate work packet type for every space (note that there needs to be one per space, rather than one per policy since spaces can maintain space-specific state).

Then a general call to traceObject() will place the respective object on the correct work queue according to which space the object resides in.

The dispatch can be optimized in a variety of ways, including an address-based scheme, where certain bits within the object address denote the space ID and the dispatch requires simply indexing into an array of space-specific work queues. This can be further improved by hiding the dispatch behind the syntactic sugar of a function call on the ObjectReference type.

@steveblackburn steveblackburn added C-enhancement Category: Enhancement C-cleanup Category: Cleanup C-refactoring Category: Refactoring A-work-queue Area: Work queue labels Feb 24, 2021
@qinsoon
Copy link
Member

qinsoon commented Feb 24, 2021

Policy-specific code should not be in the Plans. The old MMTk use the plans only for dispatch to policies (eg a switch statement dispatching to various implementations of traceObject().

We do have 'dispatch' code in the plans, but I am not aware of policy-specific code in the plans. Can you clarify which part of the code you meant?

So for something like traceObject(), there should be a separate work packet type for every space (note that there needs to be one per space, rather than one per policy since spaces can maintain space-specific state).

Then a general call to traceObject() will place the respective object on the correct work queue according to which space the object resides in.

Currently one work packet may include many object references and they could reside in different spaces (and policies). I don't quite get how space-specific work packets work. For example, when we trace one object and it references nodes in different spaces, currently we have a buffer to store all the nodes, and create a new work packet when the buffer is full. For space-specific packets, do we have multiple buffers and store nodes by spaces to different buffers, and create space-specific work packets when any of the buffer is full?

@qinsoon qinsoon added this to the 1.0 Release milestone Oct 29, 2021
qinsoon added a commit that referenced this issue Jan 9, 2022
This PR is a step towards #258 and #110. It mainly removes the plan-specific copy context, and replaces with a configurable GC worker copy context that uses policy-specific copy context (copy allocators). The `GCWorkerCopyContext` works similarly to the configurable `Mutator`:
* `GCWorkerCopyContext` ≈ `Mutator`
* `CopySemantics` ≈ `AllocationSemantics`
* `CopyConfig` ≈ `MutatorConfig`

This PR
* renames `CopyContext` (plan-specific) to `PolicyCopyContext`, and moves it to `policy/copy_context`.
  * only copy policies provide implementation for it, namely `CopySpaceCopyContext` and `ImmixCopyContext`.
* adds a `GCWorkerCopyContext` which is a combination of `PolicyCopyContext`, and will be used as the thread local data structure for a GC worker.
  * `CopySemantics` is introduced.
  * A plan needs to provide a `CopyConfig` that maps `CopySemantics` to copy allocators, and map copy allocators to spaces.
  * For copy operation, a proper `CopySemantics` need to be provided.
  * `GCWorkerLocalPtr` and a few type parameters about the `CopyContext` is removed, as now GC worker local and the copy context has the fixed type of `GCWorkerCopyContext`.
* removes the plan specific copy context (such as `SSCopyContext`, `GenCopyCopyContext`, etc)
qinsoon added a commit that referenced this issue Feb 17, 2022
This PR adds a general implementation for `ProcessEdges`, called `SFTProcessEdges`. It uses `SFT.sft_trace_object()` for each policy. A plan does not need to implement their own process edges work packet if they use `SFTProcessEdges`. This PR greatly simplifies the GC work implementation for most GC plans. This PR closes #110, and it is an important step towards #258.

Currently only Immix (including GenImmix) and mark compact uses custom process edges. All the other plans use `SFTProcessEdges`.

This PR
* adds `SFT.sft_trace_object()`.
* adds `Space.set_copy_for_sft_trace()` to set copy context for each space, which is used when we invoke `sft_trace_object()`.
* adds `SFTProcessEdges`, and use it for most plans (except immix/genimmix and mark compact).
@qinsoon
Copy link
Member

qinsoon commented Feb 20, 2022

Most of this issue is done by #542.

There are some leftover work for mark compact and immix (including gen immix). Both mark compact and immix are still using their own trace object implementation. We should do some refactoring for them with the goal of moving policy-specific code out of plan. The idea is simple: policy-specific code should not be in the Plans. We should consider letting policies provide GC work packets and use those work packets in the plan.

Space-specific work packet is an optimisation. We possibly want to track it in another issue if the rest is done for this issue and the issue can be closed.

qinsoon added a commit that referenced this issue May 13, 2022
This PR adds `PlanProcessEdges`, and a few macros/traits that are used in `PlanProcessEdges`. This PR removes policy-specific code from plan, and use macros to generate trace object work for each plan. This PR closes #258, and closes #576.

* add a trait `PolicyTraceObject`. Each policy provides an implementation of this. With this trait, a plan no longer include any policy-specific code in trace object.
* add a trait `PlanTraceObject` and related macros. With the macro, plan implementer can declaratively specify trace object behavior with attributes, and the implementation of `PlanTraceObject` will be generated by the macro.
* add a type `PlanProcessEdges`. This uses `PlanTraceObject` to provide a general implementation of `ProcessEdgesWork`. We no longer need any plan-specific process edges work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-work-queue Area: Work queue C-cleanup Category: Cleanup C-enhancement Category: Enhancement C-refactoring Category: Refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants