Bug 214231 - [JSC] Avoid JSString creation in Intl.Locale#{minimize,maximize}
Summary: [JSC] Avoid JSString creation in Intl.Locale#{minimize,maximize}
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-11 23:51 PDT by Yusuke Suzuki
Modified: 2020-07-12 13:29 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.73 KB, patch)
2020-07-11 23:51 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (3.74 KB, patch)
2020-07-12 00:02 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-07-11 23:51:13 PDT
[JSC] Avoid JSString creation in Intl.Locale#{minimize,maximize}
Comment 1 Yusuke Suzuki 2020-07-11 23:51:56 PDT
Created attachment 404088 [details]
Patch
Comment 2 Yusuke Suzuki 2020-07-12 00:02:19 PDT
Created attachment 404089 [details]
Patch
Comment 3 Yusuke Suzuki 2020-07-12 13:07:43 PDT
Comment on attachment 404089 [details]
Patch

Thanks!
Comment 4 EWS 2020-07-12 13:14:04 PDT
Committed r264285: <https://trac.webkit.org/changeset/264285>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404089 [details].
Comment 5 Radar WebKit Bug Importer 2020-07-12 13:15:19 PDT
<rdar://problem/65441234>
Comment 6 Darin Adler 2020-07-12 13:21:30 PDT
Comment on attachment 404089 [details]
Patch

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

> Source/JavaScriptCore/runtime/IntlLocale.cpp:221
>      String tag = tagValue.inherits<IntlLocale>(vm) ? jsCast<IntlLocale*>(tagValue)->toString() : tagValue.toWTFString(globalObject);

Is calling IntlLocale::toString better than just calling toWTFString on the tag value? Is the special case here important for correctness? Performance?
Comment 7 Yusuke Suzuki 2020-07-12 13:29:05 PDT
Comment on attachment 404089 [details]
Patch

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

>> Source/JavaScriptCore/runtime/IntlLocale.cpp:221
>>      String tag = tagValue.inherits<IntlLocale>(vm) ? jsCast<IntlLocale*>(tagValue)->toString() : tagValue.toWTFString(globalObject);
> 
> Is calling IntlLocale::toString better than just calling toWTFString on the tag value? Is the special case here important for correctness? Performance?

This is important for correctness, and also improves the performance.
According to the spec, here is special case. https://tc39.es/ecma402/#sec-Intl.Locale

8. If Type(tag) is Object and tag has an [[InitializedLocale]] internal slot, then
8.a Let tag be tag.[[Locale]].

This means that even if the given locale object has its own `toString` user defined method, initializeLocale should ignore calling this function and instead retrieve toString result from Locale's field directly.
And it also improves the performance since we do not need to invoke `toString` JS function here.