Repository navigation
Replace VmObjectLike.iterateMemberValues() with VmObjectCursor - #1879
Merged
Merged
Conversation
bioball
force-pushed
the
object-cursor
branch
2 times, most recently
from
September 25, 2026 20:10
af44871 to
54d475d
Compare
HT154
reviewed
Sep 25, 2026
HT154
approved these changes
Sep 30, 2026
HT154
reviewed
Oct 1, 2026
| ^^^^ | ||
| at undefinedProperty7#Person.name (file:///$snippetsDir/input/errors/undefinedProperty7.pkl) | ||
|
|
||
| The above error occurred when rendering path `name` of module `file:///$snippetsDir/input/errors/undefinedProperty7.pkl`. |
Contributor
There was a problem hiding this comment.
Why is this here?
The test input explicitly says:
// we don't expect a path to be shown in the error message, because the renderer's traverser cannot
// tell if the thrown error came from reading
Member
Author
There was a problem hiding this comment.
Yeah, good point; this is definitely wrong.
Comment on lines
+143
to
+150
| var oldFlags = flags; | ||
| if (recurse) { | ||
| if (isDeepForced()) return; | ||
| flags |= (DEEP_FORCE_FLAG | SHALLOW_FORCE_FLAG); | ||
| } else { | ||
| if (isShallowForced()) return; | ||
| flags |= SHALLOW_FORCE_FLAG; | ||
| } |
Contributor
There was a problem hiding this comment.
There are some regressions on how objects are read:
local m1 = new Mapping { ["a"] = 1; ["b"] = 2 }
m2 = (m1) {
// false here, true on main
["any"] = m2.any((_, v) -> v == 2)
// false here, true on main
["containsValue"] = m2.containsValue(2)
}
m3 = (m1) {
// true here, false on main
["isEmpty"] = m3.isEmpty
}
local l1 = new Listing { 1; 2 }
l2 = (l1) {
// false here, true on main
l2.contains(2)
// false here, true on main
l2.any((it) -> it == 1)
}
Member
Author
There was a problem hiding this comment.
Ah, great catch. The issue here was in the tracking of the flags on VmObject; m2.any is calls during a shallow force of m2, and returns a cached object cursor.
stackoverflow
approved these changes
Oct 15, 2026
stackoverflow
left a comment
Contributor
There was a problem hiding this comment.
Looks good. But the native build is failing:
=== Found 297 compilation blocklist violations ===
Probably because some @TruffleBoundary was removed in places where it's needed.
Motivation:
- VmObjectLike.iterateMemberValues() is an internal iterator whose loop body is a Java lambda.
It suffers from the following limitations:
- `continue` and `break` cannot be used.
- `return` becomes an (awkward) replacement for `continue` and `break`.
It cannot be used to return from the enclosing method.
- Mutating variables requires hoisting them (MutableBoolean, etc.).
- Node.execute() calls are very slow because they are behind a Truffle boundary.
- Callers of iterateMemberValues() do not specify what and how they want to iterate.
This obscures the iteration code and makes it difficult (often impossible) to optimize.
Changes:
- Add interface VmObjectCursor, an external iterator that iterates over VmObject members.
Its design is inspired by org.graalvm.collections.MapCursor.
- Add VmObjectCursor implementations specialized for different members, VmObject subclasses, and cursor options.
- Replace VmObjectLike.iterateMemberValues() with the following methods that return VmObjectCursor:
- properties()
- properties(CursorOption)
- elements()
- elements(CursorOption)
- entries()
- entries(CursorOption)
- members
- members(CursorOption)
- Replace all usages of iterateMemberValues() with one of the above methods.
Before: object.iterateMembersValues((key, member, value) -> { if (member.isProp()) {...} });
After : for (var cursor = object.properties(); cursor.advance(); ) {...}
Result:
- Iteration code is much simpler and clearer.
- Iteration code that calls Node.execute() is much faster because it is no longer behind a Truffle boundary.
- Other iteration code is potentially faster and can be better optimized in the future.
Co-authored-by: Jen Basch <jbasch94@gmail.com>
bioball
force-pushed
the
object-cursor
branch
from
October 16, 2026 17:32
0973e0e to
0ba4be1
Compare
bioball
enabled auto-merge (squash)
October 16, 2026 17:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Special thanks to @odenix for doing most of the work here!
This changes how object evaluation occurs.
Namely, this uses a
VmObjectCursorAPI for iteration, while actual object evaluation is lifted out of truffle boundaries and made available for partial evaluation.Motivation:
continueandbreakcannot be used.returnbecomes an (awkward) replacement forcontinueandbreak. It cannot be used to return from the enclosing method.Changes:
LoopConditionProfilein various nodes to track iterations and help truffle JIT; we can do this now because this code is now within PE-friendly regions.Result:
[native-pkl-cli-linux-amd64]