Fix write barrier parameter type #1130
Conversation
Change the target type of write barrier functions to `Option<ObjectReference>`. This allows VM bindings to use the write barrier API when storing NULL references or non-references values to slots. Fixes: mmtk#1129
Upstream PR: mmtk/mmtk-core#1130
wks
changed the title
Upstream PR: mmtk/mmtk-core#1130
|
binding-refs |
|
It is now ready for review. Binding PRs: Only the OpenJDK and the Julia bindings are affected. The master branch of the Ruby binding does not yet contain StickyImmix support, but I tested it offline. JikesRVM and V8 currently do not use write barriers when using Rust MMTk. |
src/memory_manager.rs
Outdated
| /// | ||
| /// VM bindings should use `object_reference_write_pre` and `object_reference_write_post` instead. | ||
| /// Those functions leaves the actual write of the slot to the VM binding. | ||
| #[deprecated = "Use `object_reference_write_pre` and `object_reference_write_post` instead"] |
There was a problem hiding this comment.
I don't recall we have any discussion about removing the subsuming barrier. If I missed any previous discussion, just let me know.
If I understand you right, there are a few issues with this method right now:
target: ObjectReferencecannot represent all the cases. If MMTk does the write for the binding,targetneeds to be able to represent any value the runtime may hold.Option<ObjectReference>is not sufficient either. We may want something liketarget: Value.slot: VM::VMEdgecannot handle writing values that are not references. ThoughEdge(we want to rename it toSlot) is introduced for updating slots for copying GC, I think it is a valid case of reusing this trait to update slots in write barriers. We could extend the trait to allow it to store any value.
Is there any fundamental reason that we should remove the subsuming barrier instead of trying to correct it?
The reason why write_pre/post work is that we currently do not use the slot argument, and we do not care the target value if it is not a reference (so Option<ObjectReference> is sufficient).
There was a problem hiding this comment.
We don't remove the subsuming barrier, but we need to redesign it. That's why I said "This form of write barrier has multiple issues", not subsuming barriers in general. We will replace this object_reference_write with another function, maybe with the same name, but different parameters.
There are ways to allow the VM customize the write operation of the subsuming barrier. One way is letting the caller givt it a callback that actually writes to the field so that it can write the field in whatever way it likes. The subsuming barrier effectively executes the pre and post barrier between the callback.
There was a problem hiding this comment.
I see. The attribute deprecated mostly means the function will be removed in the future, thus the use of this function is not encouraged now. Clearly this is different from this case. I think either we make a practical change to this function so it still works, or we mark it as deprecated but make it very clear that this function or the subsuming barrier will not be removed but will be redesigned, we suggest not using it before the redesign.
There was a problem hiding this comment.
I changed the comment and emphasized that we would redesign that function and replace the current one.
Emphasize that the subsuming write barrier will not be removed.
Upstream PR: mmtk/mmtk-core#1130 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Upstream PR: mmtk/mmtk-core#1130 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Upstream PR: mmtk/mmtk-core#1130 --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
2 participants
Change the target type of write barrier functions to
Option<ObjectReference>. This allows VM bindings to use the write barrier API when storing NULL references or non-references values to slots.Fixes: #1129