Bug 184015 – API::SharedJSContext caches its JSContext for at most one second

• Status: NEW
• Component: WebKit API

The -[WKWebView evaluateJavaScript:completionHandler:] API and any other APIs that internally use API::SerializedScriptValue (see APISerializedScriptValueCocoa.mm) use a JSContext to convert a JSValueRef to an Objective-C Foundation object. Creating the JSContext on an iPhone X with iOS 11.2 takes about 12~16ms and is significant relative to the other work API::SerializedScriptValue does.

The SharedJSContext class caches a JSContext to mitigate this overhead. However, the cache has a TTL of one second. This means that if one second elapses between calls to -[WKWebView evaluateJavaScript:completionHandler:], the previously cached JSContext is discarded and we pay the cost of creating a new one as well as a new JSVirtualMachine. You can observe this by timing sequential calls to -[WKWebView evaluateJavaScript:completionHandler:] -- they are generally under 1ms except for once a second when there is an extra 12~16ms.

If SharedJSContext were to cache its JSContext indefinitely (e.g., until a memory pressure event) or to cache the underlying JSVirtualMachine, it seems likely that the latency of WKWebView's calls would decrease and become more consistent, and noticeably improve the responsiveness of apps that make these calls in response to UI events.

[Alexey Proskuryakov]

The code is in WebKit/UIProcess/API/Cocoa/APISerializedScriptValueCocoa.mm. Seems like the lifetime should be extended by each use?

[Radar WebKit Bug Importer]

<rdar://problem/38927316>

It would also be desirable for JS evaluation to be responsive if the app has been idle for a while. For example, if UITouch events invoke -[WKWebView evaluateJavaScript:completionHandler:] and wait for a result via the completionHandler before updating the screen, using a cached JSContext could save a frame or two, assuming 60 FPS. So, it'd be useful to keep the JSContext around for a longer period of time until an event like memory pressure or backgrounding the app occurs.

[Ryosuke Niwa]

It's a bit strange that we have a random eviction policy of one second. It makes more sense to keep it alive as long as WKWebView is alive until there is some kind of memory pressure.

Keith / Saam, is there a reason SharedJSContext "expires" after one second?

[Saam Barati]

(In reply to Ryosuke Niwa from comment #4)
> It's a bit strange that we have a random eviction policy of one second. It
> makes more sense to keep it alive as long as WKWebView is alive until there
> is some kind of memory pressure.
> 
> Keith / Saam, is there a reason SharedJSContext "expires" after one second?

I don't know of any reason.

Geoff, do you know?

[Geoffrey Garen]

> > Keith / Saam, is there a reason SharedJSContext "expires" after one second?
> 
> I don't know of any reason.
> 
> Geoff, do you know?

It looks like I asked for some limit on this cache in bug 146416. Given that a JS VM retains a bunch of memory, I think it's an important goal that a WKWebView client should be able to allocate a WebView, do some work with it, and then throw it away without retaining a JS VM forever.

Here are some solutions that I think would be reasonable:

(1) Make allocation of a JSContext faster than 16ms. (Jeepers!)

(2) Change API::SerializedScriptValue::deserialize to accept a WKWebView argument, and keep a weak reference to that argument, and clear its cache when the set of all WebViews that has ever called API::SerializedScriptValue::deserialize becomes empty.

I think we should probably do both of these.

[Geoffrey Garen]

...I think it's also OK to increase the timer to something like 10s or 30s -- but it's not clear that would fully solve the performance complaint here.

I don't agree with the suggestion to rely on a memory pressure event. Caches that only evict upon memory pressure create a tragedy of the commons where memory pressure events become more common, and then you just always evict all of your caches all of the time. It is preferable, when possible, to evict eagerly when you know you don't need your cache anymore.

[Keith Miller]

(In reply to Geoffrey Garen from comment #7)
> ...I think it's also OK to increase the timer to something like 10s or 30s
> -- but it's not clear that would fully solve the performance complaint here.
> 
> I don't agree with the suggestion to rely on a memory pressure event. Caches
> that only evict upon memory pressure create a tragedy of the commons where
> memory pressure events become more common, and then you just always evict
> all of your caches all of the time. It is preferable, when possible, to
> evict eagerly when you know you don't need your cache anymore.

We could also evict the cache on after some number of full GCs where it's unused.

[Geoffrey Garen]

> We could also evict the cache on after some number of full GCs where it's
> unused.

I don't think there's any mechanism that will cause an idle VM to do full GCs.

[Saam Barati]

(In reply to Geoffrey Garen from comment #6)
> > > Keith / Saam, is there a reason SharedJSContext "expires" after one second?
> > 
> > I don't know of any reason.
> > 
> > Geoff, do you know?
> 
> It looks like I asked for some limit on this cache in bug 146416. Given that
> a JS VM retains a bunch of memory, I think it's an important goal that a
> WKWebView client should be able to allocate a WebView, do some work with it,
> and then throw it away without retaining a JS VM forever.
> 
> Here are some solutions that I think would be reasonable:
> 
> (1) Make allocation of a JSContext faster than 16ms. (Jeepers!)
I like this idea.

> 
> (2) Change API::SerializedScriptValue::deserialize to accept a WKWebView
> argument, and keep a weak reference to that argument, and clear its cache
> when the set of all WebViews that has ever called
> API::SerializedScriptValue::deserialize becomes empty.
> 
> I think we should probably do both of these.

[Keith Miller]

(In reply to Geoffrey Garen from comment #9)
> > We could also evict the cache on after some number of full GCs where it's
> > unused.
> 
> I don't think there's any mechanism that will cause an idle VM to do full
> GCs.

I thought we have a GC timer that occasionally fires?

[Geoffrey Garen]

> > > We could also evict the cache on after some number of full GCs where it's
> > > unused.
> > 
> > I don't think there's any mechanism that will cause an idle VM to do full
> > GCs.
> 
> I thought we have a GC timer that occasionally fires?

We have a GC timer. It fires in response to activity. After a VM goes idle, you can expect the GC timer to fire one last time, but you can't expect it to fire a "number" of times -- unless that number is zero or one :P.