Refactor: Change ActivePlan::mutators()'s return type

[ArberSephirotheca]

Close #784

mmtk/ChangeLog:
	* rust-toolchain: Update rust-toolchain to nightly version for RPITIT.
	* src/lib: Update crate attribute.
	* vm/active_plan.rs: Remove SynchronizedMutatorIterator and related trait function.
	  Change the return type of mutators function.

[qinsoon]

binding-refs
OPENJDK_BINDING_REPO=wks/mmtk-openjdk
OPENJDK_BINDING_REF=update-mmtk-core-817-alternative
JIKESRVM_BINDING_REPO=qinsoon/mmtk-jikesrvm
JIKESRVM_BINDING_REF=update-mmtk-core-817
JULIA_BINDING_REPO=qinsoon/mmtk-julia
JULIA_BINDING_REF=update-mmtk-core-817

[wks]

I suggest a different API. Instead of creating an iterator, we let the VM binding call back to MMTk core for each mutator.

trait ActivePlan<VM> {
    fn mutators<F>(visit_mutator: F)
    where
        F: FnMut(&mut MMTkMutator<VM>);
}

It is basically the classic internal iteration vs external iteration problem. An Iterator<T> is an external iterator because the user (mmtk-core) drives the iteration, while my proposed API above is an internal iterator because the container (the VM binding that holds the mutators) drives the iteration.

• OpenJDK provides an external iterator JavaThreadIteratorWithHandle.
• JikesRVM uses a global array to hold threads, so it is trivial to iterate in either way.
• The Julia binding currently holds Mutator structs in a global Rust Vec, so it is trivial to iterate in either way.
• Ruby provides an internal iteration macro ccan_list_for_each. It is internally implemented as a for loop, letting the user supply the block:

    rb_thread_t *th = NULL;
    ccan_list_for_each(&main_ractor->threads.set, th, lt_node) {
        // Each thread is assigned to th.  Visit th here.
    }

The advantage of external iteration is, Iterator<T> is more friendly to the user (mmtk-core) as it can be used together with other iteration tools in Rust such as .map(), .collect(), .zip(), .enumerate(), .filter(), etc.

The advantages of internal iteration are

1. It gives the container more control over the iteration, including the necessary operations to be done before or after the iteration, including holding and releasing some locks.
2. It is more generally implementable. In general, it is easy to convert an external iterator into an internal iteration API, but the reverse is hard (if possible).

For example, given an external iterator JavaThreadIteratorWithHandle, we can convert it into internal iteration (call-back style):

void visit_mutators(void (*visit_mutator)(Mutator *mutator)) {
    JavaThreadIteratorWithHandle jtiwh;  // Create external iterator
    JavaThread *thr;
    while ((thr = jtiwh->next()) != nullptr) {
        visit_mutator(&thr->third_part_heap_mutator); // Call back
    }
}

But converting internal iterator to external iterator is not easy. For example, given Ruby's ccan_list_for_each:

void rb_mmtk_get_mutators(void (*visit_mutator)(RubyMutator *mutator)) {
    rb_thread_t *th = NULL;
    ccan_list_for_each(&main_ractor->threads.set, th, lt_node) {
        visit_mutator(th->mutator);
    }
}

The call site of visit_mutator visits the mutator, but it is in the middle of the execution of rb_mmtk_get_mutators. So it cannot return a mutator like Iterator::next does. To adapt Ruby's current API, I implemented the visit_mutator above in Rust to collector mutators in a Rust Vec, and make an iterator that iterates through the Vec:

impl ActivePlan<Ruby> for VMActivePlan {
    fn mutators() -> Box<dyn Iterator<...>> {
        // Collect the mutator pointers
        let mut mutators = vec![];
        (upcalls().get_mutators)(|mutator_ptr| {  // For clearness, I just write it as a closure.  In the real code, it needs to be an extern "C" fn.
            mutators.push(mutator_ptr);
        });

        // Wrap the vector into an iterator struct.
        RubyMutatorIterator {
            mutator_pointer: mutators,
            cursor: 0,
        }
    }
}

It works for now.

But I think we should discuss this internal vs external iterator issue in a group. I suggest we hold this PR until we have a clearer conclusion.

[qinsoon]

Using Iterator is obviously the most Rust idiomatic way for the API. I would suggest we do what Rust prefers. The goal of this change is to make the API more Rust idiomatic and more friendly to the binding implementers.

I am not seeing strong advantages that internal iterator would be better in this case in your arguments.

It gives the container more control over the iteration, including the necessary operations to be done before or after the iteration, including holding and releasing some locks.

Each binding can implement its own Iterator type, which would give them enough flexibility. With regards to locks, currently I let the iterator hold a lock guard in some bindings to prevent any change to the mutator list. That is sufficient for now. I don't know if there is any VM we plan to support that does something more complex than that (e.g. spawn/despawn mutators while iterating mutators). If so, we may need to think about whether that is doable with Iterator, or change our API.

It is more generally implementable. In general, it is easy to convert an external iterator into an internal iteration API, but the reverse is hard (if possible).

If a binding can implement the old API, they can implement the new API with Iterator. I don't think this is a big issue.

[wks]

If a binding can implement the old API, they can implement the new API with Iterator. I don't think this is a big issue.

That's true, but the Ruby binding always used the hack of storing mutator pointers into an intermediate vector in the Iterator to workaround its internal iterating API.

But anyway, the API introduced by this PR is still an improvement over the previous API that assumed a global iterator instance. I am OK with this PR now, and let's worry about the internal vs external iteration problem if we encounter a particular VM or use case where Iterator is inconvenient to implement.

[wks]

LGTM, but we may consider switching to internal iteration API (call-back style) if we encounter a VM that is inconvenient to implement external iteration of mutators using its API.