-
-
Notifications
You must be signed in to change notification settings - Fork 57
Make SwiftRuntime.memory constant property
#375
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
Conversation
SwiftRuntime.memory constant property (追記ここまで)
81b6672 to
9a6c4c1
Compare
9a6c4c1 to
d5909d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the memory management layer and updates APIs to use a constant "SwiftRuntime.memory" property by replacing older Memory implementations with a new JSObjectSpace and caching mechanisms. Key changes include:
- Renaming and refactoring of memory‐related classes and methods (e.g. SwiftRuntimeHeap → JSObjectSpace).
- Upgrading function signatures to use DataView and JSObjectSpace instead of the old Memory instance.
- Removing the obsolete Memory implementation from Runtime/src/memory.ts.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/prelude.mjs | Adds an extra parameter to assert.equal for improved error output |
| Runtime/src/object-heap.ts | Renames and refactors memory access methods (referenceHeap → getObject) |
| Runtime/src/memory.ts | Entire file removed in favor of the new JSObjectSpace-based approach |
| Runtime/src/js-value.ts | Updates function signatures to accept DataView and JSObjectSpace |
| Runtime/src/itc.ts | Updates constructor dependency from Memory to JSObjectSpace |
| Runtime/src/index.ts | Refactors multiple WASM memory accesses to utilize cached DataView and JSObjectSpace |
| Plugins/PackageToJS/Templates/runtime.mjs | Reflects similar API changes and refactor as in Runtime/src/index.ts |
Comments suppressed due to low confidence (1)
Runtime/src/index.ts:389
- Ensure that all instances converting the Memory API calls to use DataView and JSObjectSpace are consistent; consider reviewing similar calls throughout the file to verify correct behavior when WASM memory grows.
return JSValue.writeAndReturnKindBits(result, payload1_ptr, payload2_ptr, true, this.getDataView(), this.memory);
Copilot
AI
Jun 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider clarifying why the additional 'error' argument is passed to assert.equal to aid debugging; a short inline comment could help future maintainers understand its purpose.
Uh oh!
There was an error while loading. Please reload this page.
Also cache DataView instance as much as possible (until the underlying memory is grown)