RESOLVED FIXED 210920
Improve API::SerializedScriptValue::deserialize to not allocate a new JSContext every second
https://bugs.webkit.org/show_bug.cgi?id=210920
Summary Improve API::SerializedScriptValue::deserialize to not allocate a new JSConte...
Simon Taylor
Reported 2020-04-23 10:13:24 PDT
In r186229 (related to bug 146416) a static API::SerializedScriptValue::deserialize() function was introduced. This function converts from a SerializedScriptValue to an Objective C object, and is currently used in the implementation of these APIs that return results from the content process: - evaluateJavaScript:completionHandler: for WKWebView, and - userContentController:didReceiveScriptMessage: for WKScriptMessageHandler SerializedScriptValue::deserialize uses a JSContext to do the conversion, with a SharedJSContext helper to cache this between calls so it does not need to create a completely fresh one each time (this was bug 146416). However the SharedJSContext clears the cached JSContext one second after its initial creation due to this line: https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/UIProcess/API/Cocoa/APISerializedScriptValueCocoa.mm#L49 Any apps that make heavy use of passing data from the WKWebView to the native side therefore end up recreating the JSContext for the Objective C value conversion once per second. The most annoying implication of this is when debugging - this only-for-conversion JSContext shows up in Safari's Develop menu, and makes the options to automatically show inspector and pause JSContexts essentially useless. It was digging into what those were that led to this entire adventure :) There is also a small performance impact - from Instruments System Trace it looks like my iPod 7 spends around 1.5ms doing [[JSContext alloc] init]. It's only at most once per second, so not huge overall, but still would be nice to get rid of this cost. Here's my initial thoughts for improvements: - Reset the timeout to clear the shared JSContext in every ensureContext() call (so active use of evaluateJavascript will keep it around indefinitely) - Increment a ref count on the shared context for each WKScriptMessageHandler attached I don't know the memory cost of the shared JSContext, but attaching a WKScriptMessageHandler is a pretty clear signal the developer expects messages to be coming back from the WebView, so I'd argue keeping the shared context around in this case makes sense.
Attachments
Patch (3.00 KB, patch)
2022-04-06 07:53 PDT, Chris Dumez
no flags
chad
Comment 1 2022-03-19 03:16:18 PDT
Looks like a solution for this issue: WebKit::WebUserContentControllerProxy::didReceiveMessage spents long time on JSContext initailization. https://developer.apple.com/forums/thread/702626
Chris Dumez
Comment 2 2022-04-06 07:53:01 PDT
Geoffrey Garen
Comment 3 2022-04-06 10:39:03 PDT
Comment on attachment 456819 [details] Patch r=me
EWS
Comment 4 2022-04-06 11:30:53 PDT
Committed r292483 (249334@main): <https://commits.webkit.org/249334@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 456819 [details].
Radar WebKit Bug Importer
Comment 5 2022-04-06 11:31:29 PDT
Note You need to log in before you can comment on or make changes to this bug.