-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Refactor JSClosure to leverage FinalizationRegistry
#128
Changes from 2 commits
36dd08e
652eb14
c77e069
f3cf4d4
ae62488
43a33ee
033ff48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,5 @@ class Benchmark { | |
return .undefined | ||
} | ||
runner("\(title)/\(name)", jsBody) | ||
jsBody.release() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import _CJavaScriptKit | ||
|
||
fileprivate var closureRef: JavaScriptHostFuncRef = 0 | ||
fileprivate var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] | ||
|
||
/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime is manualy managed | ||
|
@@ -10,40 +11,9 @@ public protocol JSClosureProtocol: JSValueCompatible { | |
func release() | ||
} | ||
|
||
/// `JSOneshotClosure` is a JavaScript function that can be called only once. | ||
public class JSOneshotClosure: JSObject, JSClosureProtocol { | ||
private var hostFuncRef: JavaScriptHostFuncRef = 0 | ||
|
||
public init(_ body: @escaping ([JSValue]) -> JSValue) { | ||
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. | ||
super.init(id: 0) | ||
let objectId = ObjectIdentifier(self) | ||
let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) | ||
// 2. Retain the given body in static storage by `funcRef`. | ||
sharedClosures[funcRef] = { | ||
defer { self.release() } | ||
return body($0) | ||
} | ||
// 3. Create a new JavaScript function which calls the given Swift function. | ||
var objectRef: JavaScriptObjectRef = 0 | ||
_create_function(funcRef, &objectRef) | ||
|
||
hostFuncRef = funcRef | ||
id = objectRef | ||
} | ||
|
||
/// Release this function resource. | ||
/// After calling `release`, calling this function from JavaScript will fail. | ||
public func release() { | ||
sharedClosures[hostFuncRef] = nil | ||
} | ||
} | ||
|
||
/// `JSClosure` represents a JavaScript function the body of which is written in Swift. | ||
/// This type can be passed as a callback handler to JavaScript functions. | ||
/// Note that the lifetime of `JSClosure` should be managed by users manually | ||
/// due to GC boundary between Swift and JavaScript. | ||
/// For further discussion, see also [swiftwasm/JavaScriptKit #33](https://github.com/swiftwasm/JavaScriptKit/pull/33) | ||
/// | ||
/// e.g. | ||
/// ```swift | ||
|
@@ -55,12 +25,10 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol { | |
/// button.addEventListener!("click", JSValue.function(eventListenter)) | ||
/// ... | ||
/// button.removeEventListener!("click", JSValue.function(eventListenter)) | ||
/// eventListenter.release() | ||
/// ``` | ||
/// | ||
public class JSClosure: JSObject, JSClosureProtocol { | ||
private var hostFuncRef: JavaScriptHostFuncRef = 0 | ||
var isReleased: Bool = false | ||
|
||
@available(*, deprecated, message: "This initializer will be removed in the next minor version update. Please use `init(_ body: @escaping ([JSValue]) -> JSValue)` and add `return .undefined` to the end of your closure") | ||
@_disfavoredOverload | ||
|
@@ -72,33 +40,29 @@ public class JSClosure: JSObject, JSClosureProtocol { | |
} | ||
|
||
public init(_ body: @escaping ([JSValue]) -> JSValue) { | ||
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. | ||
super.init(id: 0) | ||
let objectId = ObjectIdentifier(self) | ||
let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) | ||
// 2. Retain the given body in static storage by `funcRef`. | ||
sharedClosures[funcRef] = body | ||
// 3. Create a new JavaScript function which calls the given Swift function. | ||
self.hostFuncRef = closureRef | ||
closureRef += 1 | ||
|
||
// Retain the given body in static storage by `closureRef`. | ||
sharedClosures[self.hostFuncRef] = body | ||
|
||
// Create a new JavaScript function which calls the given Swift function. | ||
var objectRef: JavaScriptObjectRef = 0 | ||
_create_function(funcRef, &objectRef) | ||
_create_function(self.hostFuncRef, &objectRef) | ||
|
||
hostFuncRef = funcRef | ||
id = objectRef | ||
super.init(id: objectRef) | ||
} | ||
|
||
public func release() { | ||
isReleased = true | ||
sharedClosures[hostFuncRef] = nil | ||
} | ||
@available(*, deprecated, message: "JSClosure.release() is no longer necessary") | ||
public func release() {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||
} | ||
|
||
deinit { | ||
guard isReleased else { | ||
// Safari doesn't support `FinalizationRegistry`, so we cannot automatically manage the lifetime of Swift objects | ||
fatalError("release() must be called on JSClosure objects manually before they are deallocated") | ||
} | ||
} | ||
@_cdecl("_free_host_function_impl") | ||
func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) { | ||
sharedClosures[hostFuncRef] = nil | ||
} | ||
|
||
|
||
// MARK: - `JSClosure` mechanism note | ||
// | ||
// 1. Create a thunk in the JavaScript world, which has a reference | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,6 @@ | |
"license": "MIT", | ||
"devDependencies": { | ||
"prettier": "2.1.2", | ||
"typescript": "^4.0.2" | ||
"typescript": "^4.2.4" | ||
} | ||
} |
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.
Could you measure runtime performance between
JSOneshotClosure
andJSClosure
for deallocation overhead?I think deallocation that depends on GC cycle will increase memory usage while waiting until event loop frame.
(But I'm not sure how this overhead affects users experience 😅 )