Skip to content

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

Merged
merged 2 commits into from
Jun 16, 2025

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Jun 16, 2025

Also cache DataView instance as much as possible (until the underlying memory is grown)

==============================
   COMPARISON WITH BASELINE
==============================

Test                                                       | Status   | Baseline (ms)   | Current (ms)    | Change          | Change (%)
----------------------------------------------------------------------------------------------------------------------------------------
Call/JavaScript function call through Wasm import          | FASTER   | 125.69          | 117.39          | -8.30 ms        | -6.60%
Call/JavaScript function call through Wasm import with int | FASTER   | 75.97           | 70.69           | -5.28 ms        | -6.95%
Property access/Write Number                               | FASTER   | 484.17          | 411.26          | -72.91 ms       | -15.06%
Property access/Read Number                                | FASTER   | 621.07          | 432.51          | -188.56 ms      | -30.36%
Property access/Write String                               | FASTER   | 919.29          | 714.24          | -205.05 ms      | -22.31%
Property access/Read String                                | FASTER   | 1712.38         | 1239.51         | -472.87 ms      | -27.61%

@kateinoigakukun kateinoigakukun changed the title Yt/optimize runtime memory get Make SwiftRuntime.memory constant property Jun 16, 2025
@kateinoigakukun kateinoigakukun force-pushed the yt/optimize-runtime-memory-get branch from 81b6672 to 9a6c4c1 Compare June 16, 2025 04:21
@kateinoigakukun kateinoigakukun force-pushed the yt/optimize-runtime-memory-get branch from 9a6c4c1 to d5909d5 Compare June 16, 2025 05:01
@kateinoigakukun kateinoigakukun requested a review from Copilot June 16, 2025 05:10
Copy link

@Copilot Copilot AI left a 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);

@@ -110,7 +110,7 @@ function BridgeJSRuntimeTests_runJsWorks(instance, exports) {
exports.throwsSwiftError();
assert.fail("Expected error");
} catch (error) {
Copy link
Preview

Copilot AI Jun 16, 2025

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.

Suggested change
} catch (error) {
} catch (error) {
// Include the 'error' object to provide additional debugging information if the assertion fails.

Copilot uses AI. Check for mistakes.

@kateinoigakukun kateinoigakukun marked this pull request as ready for review June 16, 2025 05:13
@kateinoigakukun kateinoigakukun merged commit 0199937 into main Jun 16, 2025
8 of 12 checks passed
@kateinoigakukun kateinoigakukun deleted the yt/optimize-runtime-memory-get branch June 16, 2025 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant