Bug 154982 – JSArrayBuffers should be collected less aggressively

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

JSArrayBuffers should be collected less aggressively

[Keith Miller]

Created attachment 272777 [details]
Patch

[Geoffrey Garen]

Comment on attachment 272777 [details]
Patch

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

> Source/JavaScriptCore/runtime/JSArrayBuffer.h:43
> +    enum class WrapperMode {
> +        Wrap,
> +        DontWrap
> +    };

Let's not do these modes just to avoid one virtual function call -- unless you have some benchmark data that says we need to. We're already on the slow path, so the goal is to be reliable, understandable, and correct.

> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h:-439
> -        RELEASE_ASSERT(thisObject->existingBufferInButterfly());

Why isn't this ASSERT valid? This is the kind of thing you should explain in the ChangeLog.

> Source/JavaScriptCore/runtime/SimpleTypedArrayController.h:52
> +    virtual void registerWrapper(JSGlobalObject*, ArrayBuffer*, JSArrayBuffer*) override;

Please update the comment above about "and it didn't die" -- that's not really how this works anymore.

> Source/WebCore/bindings/js/JSDOMBinding.h:-412
> -    buffer->ref();

You should explain in the ChangeLog that this ref() was balanced by a deref() in finalize(), but both were unnecessary because...

What is the other mechanism that manages this lifetime?

[Keith Miller]

Created attachment 272795 [details]
Patch

[Geoffrey Garen]

Comment on attachment 272795 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 272795 [details]
Patch

Clearing flags on attachment: 272795

Committed r197543: <http://trac.webkit.org/changeset/197543>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.