Bug 144599 - Add a fast path for simple Weak finalization.
Summary: Add a fast path for simple Weak finalization.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: Performance
Depends on:
Blocks:
 
Reported: 2015-05-04 15:08 PDT by Andreas Kling
Modified: 2015-05-04 20:51 PDT (History)
1 user (show)

See Also:


Attachments
Patch idea (8.50 KB, patch)
2015-05-04 15:09 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2015-05-04 15:08:14 PDT
Hack from weekend: fast path for finalizing weaks that really just want to clear out a Weak.
Comment 1 Andreas Kling 2015-05-04 15:09:06 PDT
Created attachment 252338 [details]
Patch idea
Comment 2 Geoffrey Garen 2015-05-04 19:37:11 PDT
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?
Comment 3 Andreas Kling 2015-05-04 20:51:26 PDT
Comment on attachment 252338 [details]
Patch idea

Back to drawing board to address Geoff's comments.