[webkit-reviews] review granted: [Bug 172413] [JSC] Map and Set constructors should have fast path for cloning : [Attachment 310976] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 27 15:43:12 PDT 2017


Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 172413: [JSC] Map and Set constructors should have fast path for cloning
https://bugs.webkit.org/show_bug.cgi?id=172413

Attachment 310976: Patch

https://bugs.webkit.org/attachment.cgi?id=310976&action=review




--- Comment #26 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 310976
  --> https://bugs.webkit.org/attachment.cgi?id=310976
Patch

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

Nice. r=me

> Source/JavaScriptCore/runtime/HashMapImpl.h:526
> +    ALWAYS_INLINE void addNormalizedInternal(ExecState* exec, JSValue key,
JSValue value, const CanUseBucket& canUseBucket)

Nit: I’d add ASSERT(key == normalizedKey(key))

> Source/JavaScriptCore/runtime/JSMap.cpp:75
> +	   if (structure == globalObject->mapStructure())

Nit: this check is essentially no faster than the below branch. Perhaps it can
be removed in favor of just the branch below. I understand the check in the
nice case since it prevents a hahsmap lookup. But that’s not the case here.

> Source/JavaScriptCore/runtime/JSSet.cpp:75
> +	   if (structure == globalObject->setStructure())

Ditto


More information about the webkit-reviews mailing list