[webkit-reviews] review granted: [Bug 180916] [JSC] Use IsoSpace for JSWeakMap and JSWeakSet to use finalizeUnconditionally : [Attachment 329615] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Dec 17 12:17:52 PST 2017
Darin Adler <darin at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 180916: [JSC] Use IsoSpace for JSWeakMap and JSWeakSet to use
finalizeUnconditionally
https://bugs.webkit.org/show_bug.cgi?id=180916
Attachment 329615: Patch
https://bugs.webkit.org/attachment.cgi?id=329615&action=review
--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 329615
--> https://bugs.webkit.org/attachment.cgi?id=329615
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=329615&action=review
I see the performance benefit of this.
What is the performance cost? Will this use more memory or reduce locality of
reference in a way that will affect speed of execution?
Is there a security benefit or a security cost?
> Source/JavaScriptCore/ChangeLog:12
> + Currently we still have WeakReferenceHaverster in JSWeakMap and
JSWeakSet. We should
typo: "harvester" rather than "haverster"
> Source/JavaScriptCore/runtime/WeakMapImplInlines.h:33
> +template <typename WeakMapBucket>
No space after "template" to match the style we prefer elsewhere.
> Source/JavaScriptCore/runtime/WeakMapImplInlines.h:34
> +inline void WeakMapImpl<WeakMapBucket>::finalizeUnconditionally(VM&)
Do we really need to mark this inline? Since it’s a function template we don’t
need it for correctness, so I presume it’s needed here only because it actually
convinces some particular compiler to do the inlining.
More information about the webkit-reviews
mailing list