[Webkit-unassigned] [Bug 210920] New: Improve API::SerializedScriptValue::deserialize to not allocate a new JSContext every second

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=210920

            Bug ID: 210920
           Summary: Improve API::SerializedScriptValue::deserialize to not
                    allocate a new JSContext every second
           Product: WebKit
           Version: Safari 13
          Hardware: All
                OS: All
            Status: NEW
          Severity: Enhancement
          Priority: P2
         Component: WebKit API
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: simontaylor1 at ntlworld.com

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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200423/05d3cb5c/attachment.htm>