Skip to content

Commit 1ca595a

Browse files
committed
[MERGE #4006 @dilijev] OS#14163877: Add caches for Date.prototype.toLocaleString etc, with default params.
Merge pull request #4006 from dilijev:intl-date-toLocaleString-cache When calling `toLocaleString` functions that are implemented in Intl like `Date.prototype.toLocaleString`, that call is essentially sugar for creating an Intl.DateTimeFormat object, and formatting `this`. We naively throw away that object on the assumption that users will cache an `Intl.DateTimeFormat` object if they plan on making the same format call repeatedly. At a minimum, we can cache the formatter object created when the calls are made with no arguments passed in. In particular, these two code patterns are semantically identical: ``` let d = new Date(); let formatter = new Intl.DateTimeFormat(); print(formatter.format(d)); ``` ``` let d = new Date(); print(d.toLocaleString()); ``` This change significantly improves the running time when Date.prototype.toLocaleString is called in a loop with default options. #4007 has been opened to track further-reaching improvements related to caching behavior on implicitly-created i18n objects (applicable to both WinGlob and ICU). The remaining gap in the measurement below between ch and d8 is likely related to debug versus release and WinGlob versus ICU. ``` >eshost -h d8,ch-dev-che3,ch-master-latest caching.js #### d8 time spent 29 ms time spent 19 ms time spent 18 ms #### ch-dev-che3 # (x64_debug, with this change to add caching behavior) time spent 106 ms time spent 43 ms time spent 45 ms #### ch-master-latest # (x64_debug, baseline) time spent 2173 ms time spent 1676 ms time spent 1168 ms ``` caching.js ``` const iterations = 1000; var arr = []; function testMethod(method) { var start = new Date(); testDateMethod(method); var diff = (new Date()) - start; print('time spent ' + diff + ' ms'); } function testDateMethod(method) { arr = []; var d = new Date(); for (var i = 0; i < iterations; i++) { arr.push(method.call(d)); } return arr.length; } function test() { testMethod(Date.prototype.toLocaleString); testMethod(Date.prototype.toLocaleDateString); testMethod(Date.prototype.toLocaleTimeString); } test(); ``` No difference in runtime between calling the functions directly versus using `call`. Additionally: * Made some refactorings to improve readability of this code and similar code throughout Intl.js * Fixed #4009: Intl.DateTimeFormat().format.length should be 1 * Fixed #4012: Use an enum instead of magic constants for builtin function IDs
2 parents 1932f87 + cec5b12 commit 1ca595a

File tree

7 files changed

+15146
-14655
lines changed

7 files changed

+15146
-14655
lines changed

lib/Runtime/Library/InJavascript/Intl.js

Lines changed: 105 additions & 75 deletions
Large diffs are not rendered by default.

lib/Runtime/Library/InJavascript/Intl.js.bc.32b.h

Lines changed: 3868 additions & 3759 deletions
Large diffs are not rendered by default.

lib/Runtime/Library/InJavascript/Intl.js.bc.64b.h

Lines changed: 3868 additions & 3758 deletions
Large diffs are not rendered by default.

lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.32b.h

Lines changed: 3641 additions & 3526 deletions
Large diffs are not rendered by default.

lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.64b.h

Lines changed: 3641 additions & 3526 deletions
Large diffs are not rendered by default.

lib/Runtime/Library/IntlEngineInterfaceExtensionObject.cpp

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,41 +1703,53 @@ namespace Js
17031703
*/
17041704
Var IntlEngineInterfaceExtensionObject::EntryIntl_RegisterBuiltInFunction(RecyclableObject* function, CallInfo callInfo, ...)
17051705
{
1706+
// Don't put this in a header or add it to the namespace even in this file. Keep it to the minimum scope needed.
1707+
enum class IntlBuiltInFunctionID : int32 {
1708+
Min = 0,
1709+
DateToLocaleString = Min,
1710+
DateToLocaleDateString,
1711+
DateToLocaleTimeString,
1712+
NumberToLocaleString,
1713+
StringLocaleCompare,
1714+
Max
1715+
};
1716+
17061717
EngineInterfaceObject_CommonFunctionProlog(function, callInfo);
17071718

1708-
//This function will only be used during the construction of the Intl object, hence Asserts are in place.
1719+
// This function will only be used during the construction of the Intl object, hence Asserts are in place.
17091720
Assert(args.Info.Count >= 3 && JavascriptFunction::Is(args.Values[1]) && TaggedInt::Is(args.Values[2]));
17101721

17111722
JavascriptFunction *func = JavascriptFunction::FromVar(args.Values[1]);
17121723
int32 id = TaggedInt::ToInt32(args.Values[2]);
1724+
Assert(id >= (int32)IntlBuiltInFunctionID::Min && id < (int32)IntlBuiltInFunctionID::Max);
17131725

1714-
Assert(id >= 0 && id < 5);
17151726
EngineInterfaceObject* nativeEngineInterfaceObj = scriptContext->GetLibrary()->GetEngineInterfaceObject();
17161727
IntlEngineInterfaceExtensionObject* extensionObject = static_cast<IntlEngineInterfaceExtensionObject*>(nativeEngineInterfaceObj->GetEngineExtension(EngineInterfaceExtensionKind_Intl));
17171728

1718-
switch (id)
1729+
IntlBuiltInFunctionID functionID = static_cast<IntlBuiltInFunctionID>(id);
1730+
switch (functionID)
17191731
{
1720-
case 0:
1732+
case IntlBuiltInFunctionID::DateToLocaleString:
17211733
extensionObject->dateToLocaleString = func;
17221734
break;
1723-
case 1:
1735+
case IntlBuiltInFunctionID::DateToLocaleDateString:
17241736
extensionObject->dateToLocaleDateString = func;
17251737
break;
1726-
case 2:
1738+
case IntlBuiltInFunctionID::DateToLocaleTimeString:
17271739
extensionObject->dateToLocaleTimeString = func;
17281740
break;
1729-
case 3:
1741+
case IntlBuiltInFunctionID::NumberToLocaleString:
17301742
extensionObject->numberToLocaleString = func;
17311743
break;
1732-
case 4:
1744+
case IntlBuiltInFunctionID::StringLocaleCompare:
17331745
extensionObject->stringLocaleCompare = func;
17341746
break;
17351747
default:
1736-
Assert(false);//Shouldn't hit here, the previous assert should catch this.
1748+
AssertMsg(false, "functionID was not one of the allowed values. The previous assert should catch this.");
17371749
break;
17381750
}
17391751

1740-
//Don't need to return anything
1752+
// Don't need to return anything
17411753
return scriptContext->GetLibrary()->GetUndefined();
17421754
}
17431755

test/DebuggerCommon/ES6_intl_simple_attach.js.dbg.baseline

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@
519519
"caller": "Error <large string>",
520520
"arguments": "Error <large string>"
521521
},
522-
"length": "number 0"
522+
"length": "number 1"
523523
},
524524
"resolvedOptions": {
525525
"#__proto__": {

0 commit comments

Comments
 (0)