Bug 144599 – Add a fast path for simple Weak finalization.

• Status: NEW
• Component: JavaScriptCore

[Andreas Kling]

Hack from weekend: fast path for finalizing weaks that really just want to clear out a Weak.

[Andreas Kling]

Created attachment 252338 [details]
Patch idea

[Geoffrey Garen]

Comment on attachment 252338 [details]
Patch idea

View in context: https://bugs.webkit.org/attachment.cgi?id=252338&action=review

> Source/JavaScriptCore/heap/WeakImpl.h:48
> +    enum FinalizationMode {

enum class please.

> Source/JavaScriptCore/heap/WeakSetInlines.h:51
> +        WeakImpl** impl = static_cast<WeakImpl**>(weakImpl->context());

WeakImpl* would be an "impl". This is something else. I usually follow the Qt-ism, and call variables like this "slot".

> Source/WebCore/bindings/js/ScriptWrappableInlines.h:49
> +    m_wrapper = JSC::Weak<JSDOMWrapper>(wrapper, wrapperOwner, &m_wrapper, false);

I don't like ", false" as the indicator for this substantial change in behavior. :( Perhaps a better API would use a separate constructor, which took a WeakImpl** as an argument, signifying that you wanted the simple behavior by passing in the slot to clear. Otherwise, an enum tag should work.

It's very scary to, on the one hand, say that you want the simple behavior and, on the other hand, pass in an owner that has a finalizer with non-simple behavior. What if someone adds some code to that finalizer and expects it to run? Can we arrange to separate out a base class owner, which has no finalizer, from a subclass owner, which has one?

[Andreas Kling]

Comment on attachment 252338 [details]
Patch idea

Back to drawing board to address Geoff's comments.