Skip to content

[browser] Marshalling support float[], Span<float> and ArraySegment<float>#123642

Merged
pavelsavara merged 16 commits intodotnet:mainfrom
ArcadeMode:float-array-span-segment
Feb 19, 2026
Merged

[browser] Marshalling support float[], Span<float> and ArraySegment<float>#123642
pavelsavara merged 16 commits intodotnet:mainfrom
ArcadeMode:float-array-span-segment

Conversation

@ArcadeMode
Copy link
Contributor

@ArcadeMode ArcadeMode commented Jan 26, 2026

Fixes #97380.
Fixes #123706

Implementation is functional for CoreCLR and Mono. Added import and export tests to demonstrate.

How do I go about updating the docs? (and which version might see these changes for that matter, net10/11?)

@pavelsavara
Copy link
Member

@ArcadeMode feel free to ping me on your PRs, so that I can label it properly.

@ArcadeMode ArcadeMode marked this pull request as ready for review January 27, 2026 22:55
Copilot AI review requested due to automatic review settings January 27, 2026 22:55
@ArcadeMode
Copy link
Contributor Author

@pavelsavara to verify that i am following contribution guidelines: I have already changed and implemented an API change, is it okay to ask for API review in PR like this?

i.e. would a similar approach suffice for #97381 or should this be done through the issue or otherwise?

Copy link
Contributor

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 adds marshaling support for float[], Span<float>, and ArraySegment<float> between C# and JavaScript in the browser WASM interop layer, addressing issue #97380.

Changes:

  • Added MemoryViewType.Single enum value and corresponding Float32Array handling
  • Implemented C# marshaling methods for float arrays, spans, and array segments
  • Updated TypeScript marshaling code to handle Single type in both native and mono implementations
  • Added comprehensive test coverage for JSImport and JSExport scenarios with float types
  • Updated code generator to recognize Single type patterns

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshaled-types.ts Added MemoryViewType.Single enum and Float32Array view creation
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts Added Single to elementSizeMap with 4-byte size
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts Implemented Single type marshaling for arrays, spans, and array segments
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-cs.ts Implemented Single type unmarshaling and view type checking
src/mono/browser/runtime/marshal.ts Added Single to array_element_size function and MemoryViewType enum, refactored to if-else chains
src/mono/browser/runtime/marshal-to-js.ts Implemented Single type marshaling for arrays, spans, and array segments in mono runtime
src/mono/browser/runtime/marshal-to-cs.ts Implemented Single type unmarshaling and view type checking in mono runtime
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Single.cs Added ToJS/ToManaged methods for float[], Span, and ArraySegment
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Object.cs Added float[] handling in ToJS method for object parameters
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSMarshalerType.cs Updated validation methods to accept Single type in arrays, spans, and segments; removed extraneous blank line
src/libraries/System.Runtime.InteropServices.JavaScript/ref/System.Runtime.InteropServices.JavaScript.cs Added public API surface for float[], Span, and ArraySegment marshaling
src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSGeneratorFactory.cs Added generator patterns for Single type arrays, spans, and array segments
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JavaScriptTestHelper.cs Added test helper methods for Single type JSImport and JSExport scenarios
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSInteropTestBase.cs Added MarshalSingleArrayCases test data
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSImportTest.cs Added comprehensive tests for float arrays, spans, and array segments; improved variable naming in Double/Int32 tests
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSExportTest.cs Added JSExport tests for Span and ArraySegment

@pavelsavara
Copy link
Member

is it okay to ask for API review in PR like this?

The process is described here https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md

You need to create dedicated issue according to the issue template for API suggestion.

You can also look at successfully approved APIs

@pavelsavara
Copy link
Member

The code looks good. I added NO-MERGE label until we pass the API review.

Copilot AI review requested due to automatic review settings January 29, 2026 17:04
Copy link
Contributor

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

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

@pavelsavara
Copy link
Member

pavelsavara commented Jan 29, 2026

Please add few float signatures so that the generated code appears in src/libraries/System.Runtime.InteropServices.JavaScript/tests/JSImportGenerator.UnitTest/Compiles.cs

…System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JSImportTest.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 30, 2026 22:39
Copy link
Contributor

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

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

@ArcadeMode
Copy link
Contributor Author

@pavelsavara just a heads up that I've adressed your review comment

@stephentoub
Copy link
Member

🤖 Copilot Code Review — PR #123642

Holistic Assessment

Motivation: This PR addresses a legitimate gap in the JS interop marshalling layer. Issue #97380 and #123706 clearly document the need for float[], Span<float>, and ArraySegment<float> support for WebGPU and neural network scenarios where float is preferred over double for memory bandwidth and GPU RAM constraints.

Approach: The implementation correctly mirrors the existing double marshalling patterns. The C# marshalling code in JSMarshalerArgument.Single.cs follows the same structure as JSMarshalerArgument.Double.cs, and the TypeScript changes consistently add Single support across both Mono browser runtime and CoreCLR interop layers.

Summary: ⚠️ Needs Human Review. The core implementation appears correct and follows established patterns. However, there are a few items requiring attention before merge, and one outstanding reviewer request that should be addressed.


Detailed Findings

✅ C# Marshalling Implementation — Correct

The JSMarshalerArgument.Single.cs additions are correct:

  • ToManaged(out float[]? value) / ToJS(float[] value) correctly use Marshal.Copy with proper allocation size (value.Length * sizeof(float)) and set ElementType = MarshalerType.Single.
  • ToManaged(out ArraySegment<float> value) / ToJS(ArraySegment<float> value) correctly compute byte offsets using sizeof(float).
  • ToManaged(out Span<float> value) / ToJS(Span<float> value) follow the exact same pattern as double.

✅ TypeScript MemoryViewType Enum Alignment — Correct

Both src/mono/browser/runtime/marshal.ts and src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshaled-types.ts define:

export const enum MemoryViewType {
    Byte = 0,
    Int32 = 1,
    Double = 2,
    Single = 3,  // New
}

The values are consistent between Mono and CoreCLR implementations.

✅ fixupPointer Alignment — Correct

The TypeScript uses fixupPointer(bufferPtr, 2) for Single (4-byte alignment, 2^2 = 4) vs fixupPointer(bufferPtr, 3) for Double (8-byte alignment, 2^3 = 8). This is correct.

✅ JSMarshalerType Validation — Correct

CheckArray and CheckArraySegment in JSMarshalerType.cs now include MarshalerType.Single.

✅ Object Marshalling — Correct

JSMarshalerArgument.Object.cs properly handles float[] in both ToManaged (when slot.ElementType == MarshalerType.Single) and ToJS (when typeof(float[]) == type).

⚠️ Ref Assembly API Placement — Minor Inconsistency

The new APIs in System.Runtime.InteropServices.JavaScript.cs are split:

  • float[] APIs (lines 246-247) are correctly placed after float? and before double.
  • Span<float> and ArraySegment<float> APIs (lines 314-317) are appended at the end, after ArraySegment<double>.

This is technically fine for binary compatibility but slightly inconsistent with the grouping pattern where Span/ArraySegment of the same element type should ideally be adjacent. The existing file doesn't follow strict type ordering anyway, so this is acceptable but could be cleaned up for consistency.

⚠️ Missing Generator Unit Test Signatures — Outstanding Reviewer Request

Per @pavelsavara's review comment, float[] signatures should be added to src/libraries/System.Runtime.InteropServices.JavaScript/tests/JSImportGenerator.UnitTest/CodeSnippets.cs so that the generated marshalling code appears in Compiles.cs output. Currently line 31 has:

string[] aa1, byte[] aab, double[] aad, int[] aai

This should also include float[] aaf to validate the generator produces correct code for the new float[] marshalling support.

💡 ToManaged(out float[]?) — Consider Checking ElementType

In ToManaged(out float[]? value), the code only checks slot.Type == MarshalerType.None but doesn't verify that slot.ElementType == MarshalerType.Single. This matches the double[] implementation pattern exactly, but if JS passes a different array type, the behavior would be undefined. This is consistent with existing code, so no change needed, but worth noting for the area owner.

💡 Test Coverage — Good but Could Be Expanded

The tests cover:

  • Basic round-tripping with echo1_SingleArray, echo1_SpanOfSingle, echo1_ArraySegmentOfSingle
  • Special float values: float.Pi, float.MaxValue, float.MinValue, float.NaN, float.PositiveInfinity, float.NegativeInfinity
  • Export tests for ArraySegment<float> and Span<float>

Per @pavelsavara's review comment about testing "JS Array values of wrong type or JS number (double) above float.max", issue #123731 was filed to address type assertion issues separately.

✅ API Review Status

The PR correctly has NO-MERGE label pending API review approval for issue #123706. The proposed API surface matches the API proposal in the issue.


Summary

The implementation is well-structured and follows established patterns correctly. Before merge:

  1. Required: API review approval for issue [API Proposal]: JavaScript marshalling support float[], Span<float> and ArraySegment<float> #123706
  2. Should address: Add float[] signature to CodeSnippets.cs generator unit tests per reviewer request

The code itself is sound and ready for merge once these items are resolved.

Copilot AI review requested due to automatic review settings February 19, 2026 14:30
@pavelsavara pavelsavara removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 19, 2026
Copy link
Contributor

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

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

@pavelsavara
Copy link
Member

/ba-g CI failures are OOM on helix (exit code 137)

@pavelsavara pavelsavara merged commit f84f009 into dotnet:main Feb 19, 2026
106 of 110 checks passed
@ArcadeMode ArcadeMode deleted the float-array-span-segment branch February 19, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript community-contribution Indicates that the PR has been added by a community member os-browser Browser variant of arch-wasm

Projects

None yet

4 participants

Comments