[Webkit-unassigned] [Bug 147602] [INTL] Implement Intl.NumberFormat.prototype.resolvedOptions ()
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 2 17:34:35 PST 2016
https://bugs.webkit.org/show_bug.cgi?id=147602
--- Comment #27 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
Thanks.
(In reply to comment #24)
> Comment on attachment 267982 [details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267982&action=review
>
> Seems like a reasonable first cut. We need to remember to do some
> performance testing and tuning on this; thereâs a lot of memory allocation
> here.
>
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:94
> > +static bool isWellFormedCurrencyCode(const String& currency)
>
> Might be worth marking some of these super-simple functions inline,
> especially if they have only one or two call sites.
Inlined.
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:100
> > +static unsigned computeCurrencySortKey(const char* currency)
>
> Ditto.
Marked inline.
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:106
> > +static unsigned extractCurrencySortKey(std::pair<const char*, unsigned>* currencyMinorUnit)
>
> Ditto.
I am not sure if it will make a difference, because we don't call this function here. We pass it to tryBinarySearch.
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:143
> > + std::array<std::pair<const char*, unsigned>, 26> currencyMinorUnits = { {
> > + { "BHD", 3 },
> > + { "BIF", 0 },
> > + { "BYR", 0 },
> > + { "CLF", 4 },
> > + { "CLP", 0 },
> > + { "DJF", 0 },
> > + { "GNF", 0 },
> > + { "IQD", 3 },
> > + { "ISK", 0 },
> > + { "JOD", 3 },
> > + { "JPY", 0 },
> > + { "KMF", 0 },
> > + { "KRW", 0 },
> > + { "KWD", 3 },
> > + { "LYD", 3 },
> > + { "OMR", 3 },
> > + { "PYG", 0 },
> > + { "RWF", 0 },
> > + { "TND", 3 },
> > + { "UGX", 0 },
> > + { "UYI", 0 },
> > + { "VND", 0 },
> > + { "VUV", 0 },
> > + { "XAF", 0 },
> > + { "XOF", 0 },
> > + { "XPF", 0 }
> > + } };
>
> I believe this will do some work every time the function is called. We need
> to make sure this actually gets compiled in a way that doesnât do the array
> initialization each time, for efficiently. Might need to eschew the use of
> std::array, or mark this as const, to achieve that.
Changed it to std::pair<const char*, unsigned>[]. I tried using std::initializer_list but got an error saying that it "does not provide a subscript operator".
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:144
> > + std::pair<const char*, unsigned>* currencyMinorUnit = tryBinarySearch<std::pair<const char*, unsigned>>(currencyMinorUnits, currencyMinorUnits.size(), computeCurrencySortKey(currency.utf8().data()), extractCurrencySortKey);
>
> Would read better with the use of auto*. Unclear on why we need to
> explicitly pass the type in when invoking the tryBinarySearch function
> template; Iâd expect it to work without specifying that explicitly.
Used auto*. It looks like we have to explicitly pass the type. Otherwise, I get this error:
/WebKit/Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:145:31: error: no matching function for call to 'tryBinarySearch'
auto* currencyMinorUnit = tryBinarySearch(currencyMinorUnits, WTF_ARRAY_LENGTH(currencyMinorUnits), computeCurrencySortKey(StringView(currency)), extractCurrencySortKey);
^~~~~~~~~~~~~~~
...
/WebKit/WebKitBuild/Debug/usr/local/include/wtf/StdLibExtras.h:240:26: note: candidate template ignored: couldn't infer template argument 'ArrayElementType'
inline ArrayElementType* tryBinarySearch(ArrayType& array, size_t size, KeyType key, ExtractKey extractKey = ExtractKey())
> Should overload computeCurrencySortKey to work on a const String& or
> StringView correctly; doing memory allocation just to convert the String to
> a CString, just to get the first three characters of the string, is not
> great.
Done.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160203/8272e298/attachment-0001.html>
More information about the webkit-unassigned
mailing list