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

Refactor ProcessEdgesBase::worker to make it safe #1040

Open
wks opened this issue Dec 6, 2023 · 0 comments
Open

Refactor ProcessEdgesBase::worker to make it safe #1040

wks opened this issue Dec 6, 2023 · 0 comments
Labels
A-work-queue Area: Work queue C-refactoring Category: Refactoring F-project Call For Participation: Student projects. These issues are available student projects. G-safety Goal: Safety P-normal Priority: Normal.

Comments

@wks
Copy link
Collaborator

wks commented Dec 6, 2023

The ProcessEdgesBase::worker field is a raw pointer *mut GCWorker<VM>. It is initialised as null when instantiating ProcessEdgesBase, but later set to the pointer to the worker in do_work(). It is used in

  • ProcessEdgesWork::trace_object: It will dispatch to the trace_object method of different spaces. But since trace_object can copy object, it needs its copy_context to do the copying. For this reason, we need to pass the worker (or the underlying CopyContext to the trace_object method of concrete spaces.
  • ProcessEdgesWork::start_or_dispatch_scan_work: If it decides to execute the ScanObjects work packet immediately after tracing all slots (Edge), it will call ScanObjects::do_work, and one parameter is the GC worker.

Obviously, raw pointers are unsafe.

The root problem is that the ProcessEdgesWork work packet is instantiated when enqueuing the slots, but the reference to the GCWorker is only available when executing the ProcessEdgesWork work packet.

There are two obvious solutions:

  1. Pass the &mut GCWorker from do_work() all the way through process_edges(), process_edge() and trace_object(). But also be ware that ProcessEdgesWork::trace_object is sometimes called directly, bypassing process_edges. For example,
fn do_work(&mut self, worker: &mut GCWorker, mmtk: &MMTK) {
    self.process_edges(worker);
}
fn process_edges(&mut self, worker: &mut GCWorker) {
    for edge in self.edges {
        self.process_edge(edge, worker);
    }
}
fn process_edge(&mut self, edge: Edge, worker: &mut GCWorker) {
    edge.store(self.trace_object(edge.load(), worker)));
}
fn trace_object(&mut self, object: ObjectReference, worker: &mut GCWorker) -> ObjectReference { ... }
  1. Create another struct that contains the slots (Edge) to process as well as a reference to GCWorker, and move the process_edges, process_edge and trace_object methods to that struct. Because the struct is constructed when a &mut GCWorker is available, the worker field (as shown below) is safe, but has a lifetime annotation. For example,
struct DoProcessEdges<'w> { // This struct has a lifetime param `'w` which means it only lives when a reference to the worker is available.
    edges: Vec<Edge>,
    worker: &'w mut GCWorker<VM>, // Borrows the reference of the worker during its lifetime.
    // other fields here.
}

impl<'w> DoProcessEdges<'w> {
    fn process_edges(&mut self) { ... }
    fn process_edge(&mut self, edge: Edge) { ... }
    fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { ... }
}

Related issues and PRs

There is a stale PR for solving the problem by adding the parameter worker: &mut GCWorker to several functions. #597

@wks wks added C-refactoring Category: Refactoring A-work-queue Area: Work queue G-safety Goal: Safety F-project Call For Participation: Student projects. These issues are available student projects. labels Dec 6, 2023
@qinsoon qinsoon added the P-normal Priority: Normal. label Jan 17, 2024
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-refactoring Category: Refactoring F-project Call For Participation: Student projects. These issues are available student projects. G-safety Goal: Safety P-normal Priority: Normal.
Projects
None yet
Development

No branches or pull requests

2 participants