-
Notifications
You must be signed in to change notification settings - Fork 77
Should we rename the Edge
trait?
#687
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
Comments
In today's meeting, we think Slot is a better name because "Edge" may refer to the content of the slot, while "slot" unambiguously refers to the address of it. |
Implementor feedback here, in case it's helpful. I'm learning MMTk while implementing support for my rust implementation of the SuperCollider interpreter. To me I use the name I didn't have an issue with Which leads me to my question - is there a situation where the structure implementing Thanks, I think MMTk is really neat! |
One use case is a situation where an object contains an array, and a slot holds a pointer to an element of that array, and the index of the element it points to is only known at run time instead of compile time. For example: struct Array {
Header header;
size_t length;
int32_t elements[];
};
struct Foo {
int blahblah;
size_t index;
void* pointer;
};
void someFunction() {
Array *ary = array_new(10000);
Foo *someObject = foo_new(...);
size_t index = read_number();
someObject->index = index;
someObject->pointer = &ary->elements[index];
} In the code above, In that case, an implementation of
To represent the field |
I appreciate the explanation, thanks very much! |
This commit changes the term "edge" to "slot" where it actually refers to a field of an object or a variable on the stack or in any global data structures. Notably, the trait `Edge` is renamed to `Slot`, and related types and methods are renamed, too. We still use the term "edge" to refer to edges in the object graph, regardless whether the edge is represented by a slot or by an object reference pointing to the target. The work packet trait `ProcessEdgesWork` and its implementations still use the term "edge". Although it was originally designed to process slots, it can be used (and is currently actually used) in node-enqueuing tracing as well as weak reference processing, in which case it is used as a provider of the `trace_object` method which traces object graph edges represented as `ObjectReference` to the target objects. Note: This commit only does renaming, and does not change the program logic. The behavior of the program should be exactly the same after this change. Fixes: #687
This commit changes the term "edge" to "slot" where it actually refers to a field of an object or a variable on the stack or in any global data structures. Notably, the trait `Edge` is renamed to `Slot`, and related types and methods are renamed, too. We still use the term "edge" to refer to edges in the object graph, regardless whether the edge is represented by a slot or by an object reference pointing to the target. The work packet trait `ProcessEdgesWork` and its implementations still use the term "edge". Although it was originally designed to process slots, it can be used (and is currently actually used) in node-enqueuing tracing as well as weak reference processing, in which case it is used as a provider of the `trace_object` method which traces object graph edges represented as `ObjectReference` to the target objects. Note: This commit only does renaming, and does not change the program logic. The behavior of the program should be exactly the same after this change. Fixes: mmtk#687
I am not planning to rename it now. I would like to discuss two things before we change the code.
The Edge trait
The
Edge
trait was introduced in #606The purpose is to support different VMs that store object references in fields differently, giving the VM a place to customise. It is defined as:
An edge can be anything that supports the following operations:
load
: load an object reference from it. Used to support any GC.store
: store an updated object reference back to it. Used to support copying GC.And it needs to support
Copy + Send
in order to be transferred between work packets, andDebug + PartialEq + Eq + Hash
to support sanity GC.So an implementation of the
Edge
trait could beA ProcessEdgesWork should hold a list of Edge instances, and process each of them. I am open to the idea of specialising work packets for each different kind of edges. I do not know whether it is more efficient to use
enum
to make a union type of different kinds of edges in a VM (such as OpenJDK) and put all of them into aVec<OpenJDKEdge>
, or scatter different edges into different work packets.What's the right name?
But recently, debates around the meaning of "object reference" let us rethink whether "edge" is the proper name of this thing.
The GC Handbook does not define "edge" in its glossary, but mentioned "edge" when defining object graph. An edge (an object graph) is "a reference from a source node or a root to a destination node".
The GC Handbook defines "field" as "a component of an object holding a reference or scalar value", and "slot" as a synonym of "field".
According to these definitions, what I defined as the
trait Edge
satisfies the definition of "edge" of the GC handbook. But atrait Edge
instance does not have to be a field or slot, because it may be used to represent a local variable root from the stack or a global root.Do we need a
Slot
orField
trait?Recently, @wenyuzhao mentioned the need to get the address of a field in order to implement field-remembering barriers. If a
trait Edge
instance is always a field, then it is easy to just add aget_address()
method so the barrier can use that to access the metadata.However, a
trait Edge
instance doesn't have to be a field, and the field-remembering barrier doesn't make sense for roots. So although we can letEdge::get_address()
return an address on the stack, it still looks out of place.Currently (even before I introduced theEdge
trait), theProcessEdgesWork
work packet does not distinguish between object field edges and root edges. Before the introduction of theEdge
trait,ProcessEdgesWork
holds aVec<Address>
where theAddress
can point to anywhere, including object fields, stack variables and global variables. Withtrait Edge
introduced, it is now aVec<VM::VMEdgeType>
, but still doesn't differentiate root edges and field edges. I am not sure if that is a problem if we merge LXR into MMTk core. It looks like as long as we have theEdge::load()
,Edge::store()
andEdge::prefetch_xxx()
methods, we can update any edge for copying GC, regardless of whether those edges are from roots or from fields.Correction: Currently, the boolean field
ProcessEdgesBase::root
is set to true if the current work packet holds root edges instead of field edges.The text was updated successfully, but these errors were encountered: