Skip to content

Commit f683048

Browse files
committed
feat: preserve identity across all return shapes
Emit a per-class extension override of _BridgedSwiftHeapObject's bridgeJSLowerReturn that routes through the identity cache, symmetric to the existing bridgeJSStackPush override. This was necessary for Optional<SwiftIdentityClass> returns: the _BridgedAsOptional bridge calls value.bridgeJSLowerReturn() on the inner heap object, which previously hit the default implementation (passRetained + bare pointer) and bypassed our per-class identity table. With this override, scalar, array-element, and Optional returns all route through the same identity handshake. Before the fix, .some(x) returned a fresh wrapper each call; now it preserves === identity. Side benefit: the per-method `case .swiftHeapObject where isSwiftIdentityMode` branch in ExportedThunkBuilder.lowerReturnValue is redundant — ret.bridgeJSLowerReturn() picks up the extension override automatically. The builder's isSwiftIdentityMode predicate is removed; codegen is simpler. The upgraded E2E test now asserts === identity for Optional.some. 165/165 tests green.
1 parent 37fbaf0 commit f683048

8 files changed

Lines changed: 166 additions & 204 deletions

File tree

Benchmarks/Sources/Generated/BridgeJS.swift

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,13 +1822,7 @@ nonisolated(unsafe) var _SimpleClassSwiftIdentity_identityTable: Set<UnsafeMutab
18221822
public func _bjs_SimpleClassSwiftIdentity_init(_ nameBytes: Int32, _ nameLength: Int32, _ count: Int32, _ flag: Int32, _ rate: Float32, _ precise: Float64) -> UnsafeMutableRawPointer {
18231823
#if arch(wasm32)
18241824
let ret = SimpleClassSwiftIdentity(name: String.bridgeJSLiftParameter(nameBytes, nameLength), count: Int.bridgeJSLiftParameter(count), flag: Bool.bridgeJSLiftParameter(flag), rate: Float.bridgeJSLiftParameter(rate), precise: Double.bridgeJSLiftParameter(precise))
1825-
return withExtendedLifetime(ret) {
1826-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
1827-
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
1828-
_ = Unmanaged.passRetained(ret)
1829-
}
1830-
return ptr
1831-
}
1825+
return ret.bridgeJSLowerReturn()
18321826
#else
18331827
fatalError("Only available on WebAssembly")
18341828
#endif
@@ -1961,6 +1955,16 @@ public func _bjs_SimpleClassSwiftIdentity_release_wrapper(_ pointer: UnsafeMutab
19611955
}
19621956

19631957
extension SimpleClassSwiftIdentity {
1958+
@_spi(BridgeJS) @_transparent
1959+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
1960+
return withExtendedLifetime(self) {
1961+
let ptr = Unmanaged.passUnretained(self).toOpaque()
1962+
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
1963+
_ = Unmanaged.passRetained(self)
1964+
}
1965+
return ptr
1966+
}
1967+
}
19641968
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
19651969
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
19661970
let ptr = Unmanaged.passUnretained(self).toOpaque()
@@ -2001,13 +2005,7 @@ nonisolated(unsafe) var _ClassRoundtripSwiftIdentity_identityTable: Set<UnsafeMu
20012005
public func _bjs_ClassRoundtripSwiftIdentity_init() -> UnsafeMutableRawPointer {
20022006
#if arch(wasm32)
20032007
let ret = ClassRoundtripSwiftIdentity()
2004-
return withExtendedLifetime(ret) {
2005-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2006-
if _ClassRoundtripSwiftIdentity_identityTable.insert(ptr).inserted {
2007-
_ = Unmanaged.passRetained(ret)
2008-
}
2009-
return ptr
2010-
}
2008+
return ret.bridgeJSLowerReturn()
20112009
#else
20122010
fatalError("Only available on WebAssembly")
20132011
#endif
@@ -2018,13 +2016,7 @@ public func _bjs_ClassRoundtripSwiftIdentity_init() -> UnsafeMutableRawPointer {
20182016
public func _bjs_ClassRoundtripSwiftIdentity_roundtripSimpleClassSwiftIdentity(_ _self: UnsafeMutableRawPointer, _ obj: UnsafeMutableRawPointer) -> UnsafeMutableRawPointer {
20192017
#if arch(wasm32)
20202018
let ret = ClassRoundtripSwiftIdentity.bridgeJSLiftParameter(_self).roundtripSimpleClassSwiftIdentity(_: SimpleClassSwiftIdentity.bridgeJSLiftParameter(obj))
2021-
return withExtendedLifetime(ret) {
2022-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2023-
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
2024-
_ = Unmanaged.passRetained(ret)
2025-
}
2026-
return ptr
2027-
}
2019+
return ret.bridgeJSLowerReturn()
20282020
#else
20292021
fatalError("Only available on WebAssembly")
20302022
#endif
@@ -2035,13 +2027,7 @@ public func _bjs_ClassRoundtripSwiftIdentity_roundtripSimpleClassSwiftIdentity(_
20352027
public func _bjs_ClassRoundtripSwiftIdentity_makeSimpleClassSwiftIdentity(_ _self: UnsafeMutableRawPointer) -> UnsafeMutableRawPointer {
20362028
#if arch(wasm32)
20372029
let ret = ClassRoundtripSwiftIdentity.bridgeJSLiftParameter(_self).makeSimpleClassSwiftIdentity()
2038-
return withExtendedLifetime(ret) {
2039-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2040-
if _SimpleClassSwiftIdentity_identityTable.insert(ptr).inserted {
2041-
_ = Unmanaged.passRetained(ret)
2042-
}
2043-
return ptr
2044-
}
2030+
return ret.bridgeJSLowerReturn()
20452031
#else
20462032
fatalError("Only available on WebAssembly")
20472033
#endif
@@ -2079,6 +2065,16 @@ public func _bjs_ClassRoundtripSwiftIdentity_release_wrapper(_ pointer: UnsafeMu
20792065
}
20802066

20812067
extension ClassRoundtripSwiftIdentity {
2068+
@_spi(BridgeJS) @_transparent
2069+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
2070+
return withExtendedLifetime(self) {
2071+
let ptr = Unmanaged.passUnretained(self).toOpaque()
2072+
if _ClassRoundtripSwiftIdentity_identityTable.insert(ptr).inserted {
2073+
_ = Unmanaged.passRetained(self)
2074+
}
2075+
return ptr
2076+
}
2077+
}
20822078
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
20832079
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
20842080
let ptr = Unmanaged.passUnretained(self).toOpaque()
@@ -2119,13 +2115,7 @@ nonisolated(unsafe) var _IdentityCacheBenchmarkSwiftIdentity_identityTable: Set<
21192115
public func _bjs_IdentityCacheBenchmarkSwiftIdentity_init() -> UnsafeMutableRawPointer {
21202116
#if arch(wasm32)
21212117
let ret = IdentityCacheBenchmarkSwiftIdentity()
2122-
return withExtendedLifetime(ret) {
2123-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
2124-
if _IdentityCacheBenchmarkSwiftIdentity_identityTable.insert(ptr).inserted {
2125-
_ = Unmanaged.passRetained(ret)
2126-
}
2127-
return ptr
2128-
}
2118+
return ret.bridgeJSLowerReturn()
21292119
#else
21302120
fatalError("Only available on WebAssembly")
21312121
#endif
@@ -2174,6 +2164,16 @@ public func _bjs_IdentityCacheBenchmarkSwiftIdentity_release_wrapper(_ pointer:
21742164
}
21752165

21762166
extension IdentityCacheBenchmarkSwiftIdentity {
2167+
@_spi(BridgeJS) @_transparent
2168+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
2169+
return withExtendedLifetime(self) {
2170+
let ptr = Unmanaged.passUnretained(self).toOpaque()
2171+
if _IdentityCacheBenchmarkSwiftIdentity_identityTable.insert(ptr).inserted {
2172+
_ = Unmanaged.passRetained(self)
2173+
}
2174+
return ptr
2175+
}
2176+
}
21772177
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
21782178
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
21792179
let ptr = Unmanaged.passUnretained(self).toOpaque()

Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift

Lines changed: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -125,15 +125,9 @@ public class ExportSwift {
125125
var abiReturnType: WasmCoreType?
126126
var externDecls: [DeclSyntax] = []
127127
let effects: Effects
128-
/// Predicate that resolves whether a class-name corresponds to an
129-
/// `identityMode: "swift"` class. Default: always false. Threaded
130-
/// through from `ExportSwift.isSwiftIdentityMode(_:)` at construction
131-
/// time so return-lowering can emit the per-class cache glue.
132-
let isSwiftIdentityMode: (String) -> Bool
133128

134-
init(effects: Effects, isSwiftIdentityMode: @escaping (String) -> Bool = { _ in false }) {
129+
init(effects: Effects) {
135130
self.effects = effects
136-
self.isSwiftIdentityMode = isSwiftIdentityMode
137131
}
138132

139133
private func append(_ item: CodeBlockItemSyntax) {
@@ -351,27 +345,13 @@ public class ExportSwift {
351345
}
352346
"""
353347
)
354-
case .swiftHeapObject(let className) where isSwiftIdentityMode(className):
355-
// identityMode: "swift" — Swift's per-class Set tracks which
356-
// pointers have been retained. On miss, retain once; on hit,
357-
// skip. No signalling to JS needed: JS checks its own
358-
// `Map<pointer, wrapper>` and derives hit/miss symmetrically.
359-
// (See DECISIONS.md D20 for why dropping freshBit is safe:
360-
// the Swift Set and the JS Map stay in lockstep by
361-
// construction — both are keyed by pointer and updated at
362-
// the same cache boundaries.)
363-
append(
364-
"""
365-
return withExtendedLifetime(ret) {
366-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
367-
if _\(raw: className)_identityTable.insert(ptr).inserted {
368-
_ = Unmanaged.passRetained(ret)
369-
}
370-
return ptr
371-
}
372-
"""
373-
)
374348
default:
349+
// For `@JS(identityMode: .swift)` classes, the per-class
350+
// `bridgeJSLowerReturn` override emitted in
351+
// `renderSingleExportedClass` routes through the identity
352+
// table. Array-element and Optional paths go through the same
353+
// override (plus `bridgeJSStackPush`), so every return shape
354+
// preserves `===` identity.
375355
append("return ret.bridgeJSLowerReturn()")
376356
}
377357
}
@@ -506,8 +486,7 @@ public class ExportSwift {
506486
let isStatic = context.isStatic
507487

508488
let getterBuilder = ExportedThunkBuilder(
509-
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic),
510-
isSwiftIdentityMode: isSwiftIdentityMode
489+
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic)
511490
)
512491

513492
if !isStatic {
@@ -528,8 +507,7 @@ public class ExportSwift {
528507
// Generate property setter if not readonly
529508
if !property.isReadonly {
530509
let setterBuilder = ExportedThunkBuilder(
531-
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic),
532-
isSwiftIdentityMode: isSwiftIdentityMode
510+
effects: Effects(isAsync: false, isThrows: false, isStatic: isStatic)
533511
)
534512

535513
// Lift parameters based on property type
@@ -559,7 +537,7 @@ public class ExportSwift {
559537
}
560538

561539
func renderSingleExportedFunction(function: ExportedFunction) throws -> DeclSyntax {
562-
let builder = ExportedThunkBuilder(effects: function.effects, isSwiftIdentityMode: isSwiftIdentityMode)
540+
let builder = ExportedThunkBuilder(effects: function.effects)
563541
for param in function.parameters {
564542
try builder.liftParameter(param: param)
565543
}
@@ -588,7 +566,7 @@ public class ExportSwift {
588566
callName: String,
589567
returnType: BridgeType
590568
) throws -> DeclSyntax {
591-
let builder = ExportedThunkBuilder(effects: constructor.effects, isSwiftIdentityMode: isSwiftIdentityMode)
569+
let builder = ExportedThunkBuilder(effects: constructor.effects)
592570
for param in constructor.parameters {
593571
try builder.liftParameter(param: param)
594572
}
@@ -602,7 +580,7 @@ public class ExportSwift {
602580
ownerTypeName: String,
603581
instanceSelfType: BridgeType
604582
) throws -> DeclSyntax {
605-
let builder = ExportedThunkBuilder(effects: method.effects, isSwiftIdentityMode: isSwiftIdentityMode)
583+
let builder = ExportedThunkBuilder(effects: method.effects)
606584
if !method.effects.isStatic {
607585
try builder.liftParameter(param: Parameter(label: nil, name: "_self", type: instanceSelfType))
608586
}
@@ -772,12 +750,23 @@ public class ExportSwift {
772750
decls.append(DeclSyntax(releaseDecl))
773751
}
774752

775-
// Override the default `_BridgedSwiftHeapObject.bridgeJSStackPush`
776-
// so array-element returns (`[SwiftCached]`) go through the same
777-
// identity-cache handshake. Post-D20: no `freshBit` push — JS
778-
// checks its own Map for hit/miss.
779-
let stackPushExt: DeclSyntax = """
753+
// Override the default `_BridgedSwiftHeapObject` return/push paths
754+
// so every return type that bottoms out at a heap-object pointer —
755+
// scalar, array element, Optional — goes through the identity-cache
756+
// handshake. All three paths converge on the same per-class Set
757+
// check + conditional retain.
758+
let swiftIdentityOverridesExt: DeclSyntax = """
780759
extension \(raw: klass.swiftCallName) {
760+
@_spi(BridgeJS) @_transparent
761+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
762+
return withExtendedLifetime(self) {
763+
let ptr = Unmanaged.passUnretained(self).toOpaque()
764+
if _\(raw: klass.name)_identityTable.insert(ptr).inserted {
765+
_ = Unmanaged.passRetained(self)
766+
}
767+
return ptr
768+
}
769+
}
781770
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
782771
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
783772
let ptr = Unmanaged.passUnretained(self).toOpaque()
@@ -790,7 +779,7 @@ public class ExportSwift {
790779
}
791780
}
792781
"""
793-
decls.append(stackPushExt)
782+
decls.append(swiftIdentityOverridesExt)
794783
}
795784

796785
// Generate ConvertibleToJSValue extension

Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/IdentityModeSwiftClass.swift

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,7 @@ nonisolated(unsafe) var _SwiftCached_identityTable: Set<UnsafeMutableRawPointer>
55
public func _bjs_SwiftCached_init(_ nameBytes: Int32, _ nameLength: Int32) -> UnsafeMutableRawPointer {
66
#if arch(wasm32)
77
let ret = SwiftCached(name: String.bridgeJSLiftParameter(nameBytes, nameLength))
8-
return withExtendedLifetime(ret) {
9-
let ptr = Unmanaged.passUnretained(ret).toOpaque()
10-
if _SwiftCached_identityTable.insert(ptr).inserted {
11-
_ = Unmanaged.passRetained(ret)
12-
}
13-
return ptr
14-
}
8+
return ret.bridgeJSLowerReturn()
159
#else
1610
fatalError("Only available on WebAssembly")
1711
#endif
@@ -60,6 +54,16 @@ public func _bjs_SwiftCached_release_wrapper(_ pointer: UnsafeMutableRawPointer)
6054
}
6155

6256
extension SwiftCached {
57+
@_spi(BridgeJS) @_transparent
58+
public consuming func bridgeJSLowerReturn() -> UnsafeMutableRawPointer {
59+
return withExtendedLifetime(self) {
60+
let ptr = Unmanaged.passUnretained(self).toOpaque()
61+
if _SwiftCached_identityTable.insert(ptr).inserted {
62+
_ = Unmanaged.passRetained(self)
63+
}
64+
return ptr
65+
}
66+
}
6367
@_spi(BridgeJS) public consuming func bridgeJSStackPush() {
6468
let ptr: UnsafeMutableRawPointer = withExtendedLifetime(self) {
6569
let ptr = Unmanaged.passUnretained(self).toOpaque()

Sources/JavaScriptKit/Documentation.docc/Articles/BridgeJS/Exporting-Swift/Identity-Modes-For-Exported-Classes.md

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,20 @@ The key differences vs `.pointer`:
6969
```javascript
7070
const b = exports.getBuilding(); // allocates a wrapper (if fresh)
7171
b.name; // stable === with any future getBuilding() that returns the same Swift object
72-
b.release(); // mandatory for swift mode — frees the Swift heap object and the wrapper slot
72+
b.release(); // frees the Swift heap object and the wrapper slot
7373
// after release(): b.name throws "Attempted to call a member on a released Building"
7474
```
7575

7676
Double-release is a safe no-op. Static members (class-level methods, constructors) are not affected by release.
7777

78+
### Why `release()` is required
79+
80+
`.pointer` mode uses a `WeakRef` and a `FinalizationRegistry` to let the JS garbage collector reclaim wrappers whose users have dropped all references. That safety net is exactly what makes `.pointer` mode's miss path expensive — `FinalizationRegistry.register` and `new WeakRef` allocations account for ~88% of the per-miss cost. `.swift` mode removes both, which is where its performance comes from. The tradeoff is that the JS garbage collector no longer has any hook to trigger cleanup; Swift holds a strong retain until you explicitly release.
81+
82+
In practice this is the same discipline as any manually-managed resource (file handles, network sockets, native-backed buffers). Use `try { … } finally { x.release() }` for scopes that can throw, or wrap long-lived objects in application code that owns their release.
83+
84+
If you can't live with explicit release, stay on `.pointer` mode.
85+
7886
## Benchmarks (summary)
7987

8088
Full results in [`Benchmarks/results/swift-side-cache/Benchmarks.md`](../../../../../Benchmarks/results/swift-side-cache/Benchmarks.md). Baseline arm64-macOS, Swift 6.3, Node 22, release build, 500k iters per scenario, median ms:
@@ -91,8 +99,7 @@ Full results in [`Benchmarks/results/swift-side-cache/Benchmarks.md`](../../../.
9199

92100
## Known limitations
93101

94-
- **Optional<SwiftIdentityClass> identity is not preserved.** If your Swift API returns `SomeClass?`, `.some(x)` produces a fresh wrapper each call even with `.swift` mode. Lifecycle is still correct, but `a === b` where both come from an optional return will be `false`. Workaround: wrap in `[SomeClass]` (array element identity IS preserved).
95-
- **No automatic cleanup.** You must call `release()` explicitly. A future version may add a Swift-side timeout or GC-assisted cleanup.
102+
- **No GC-driven cleanup.** Wrappers live until `release()` is called. Dropping all JS references to a wrapper without releasing leaks the Swift heap object. See "Why `release()` is required" above.
96103
- **Wasm-only.** Like all of BridgeJS, identity modes only activate on `wasm32`. On the host, the macro expands to no-op code paths so your test harness can still compile.
97104

98105
## Choosing a mode

0 commit comments

Comments
 (0)