ObjectSpace specs #3299
ObjectSpace specs #3299
Conversation
…cals and at_exit handlers.
| @@ -115,9 +129,23 @@ private void visitObject(DynamicObject object, ObjectGraphVisitor visitor) throw | |||
|
|
|||
| // Visit all properties | |||
|
|
|||
| for (Property property : object.getShape().getProperties()) { | |||
| for (Property property : object.getShape().getPropertyListInternal(false)) { | |||
There was a problem hiding this comment.
Do we still need this change with the one below?
There was a problem hiding this comment.
Yes - getProperties() doesn't return HiddenKeys - took me ages to figure this out.
There was a problem hiding this comment.
Which HiddenKey do we want to traverse? Before they were fields and only traversed if they were explicitly in visitObjectGraphChildren, right?
There was a problem hiding this comment.
We want to visit all fields, whether they're instance variables or they're internal fields (like store in Array) to find all objects. Things like store are currently HiddenKey so that they aren't mistaken for instance variables.
There was a problem hiding this comment.
Right, sounds good and less error-prone
|
Nice specs! Please address my comments. |
| visitObject(context.getCoreLibrary().getGlobalVariablesObject(), visitor); | ||
| // We only visit the global variables and other global state from the root thread | ||
|
|
||
| if (thread == context.getThreadManager().getRootThread()) { |
There was a problem hiding this comment.
This will run for all fibers of the main thread. One way to only run for the initial Fiber is to compare with the root fiber of the main thread.
There was a problem hiding this comment.
Or more simply since any of the thread could do this, the caller of visitRoots could be chosen to lookup these ones.
@eregon please review