-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(core): not invoking object's toString when rendering to the DOM (#…
…39843) Currently we convert objects to strings using `'' + value` which is quickest, but it stringifies the value using its `valueOf`, rather than `toString`. These changes switch to using `String(value)` which has identical performance and calls the `toString` method as expected. Note that another option was calling `toString` directly, but benchmarking showed it to be slower. I've included the benchmark I used to verify the performance so we have it for future reference and we can reuse it when making changes to `renderStringify` in the future. Also for reference, here are the results of the benchmark: ``` Benchmark: renderStringify concat: 2.006 ns(0%) concat with toString: 2.201 ns(-10%) toString: 237.494 ns(-11741%) toString with toString: 121.072 ns(-5937%) constructor: 2.201 ns(-10%) constructor with toString: 2.201 ns(-10%) toString mono: 14.536 ns(-625%) toString with toString mono: 9.757 ns(-386%) ``` Fixes #38839. PR Close #39843
- Loading branch information
1 parent
24c0897
commit 75e22ab
Showing
4 changed files
with
163 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
104 changes: 104 additions & 0 deletions
104
packages/core/test/render3/perf/render_stringify/index.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
/** | ||
* @license | ||
* Copyright Google LLC All Rights Reserved. | ||
* | ||
* Use of this source code is governed by an MIT-style license that can be | ||
* found in the LICENSE file at https://angular.io/license | ||
*/ | ||
import {createBenchmark} from '../micro_bench'; | ||
|
||
// These benchmarks compare various implementations of the `renderStringify` utility | ||
// which vary in subtle ways which end up having an effect on performance. | ||
|
||
/** Uses string concatenation to convert a value into a string. */ | ||
function renderStringifyConcat(value: any): string { | ||
if (typeof value === 'string') return value; | ||
if (value == null) return ''; | ||
return '' + value; | ||
} | ||
|
||
/** Uses `toString` to convert a value into a string. */ | ||
function renderStringifyToString(value: any): string { | ||
if (typeof value === 'string') return value; | ||
if (value == null) return ''; | ||
return value.toString(); | ||
} | ||
|
||
/** Uses the `String` constructor to convert a value into a string. */ | ||
function renderStringifyConstructor(value: any): string { | ||
if (typeof value === 'string') return value; | ||
if (value == null) return ''; | ||
return String(value); | ||
} | ||
|
||
const objects: any[] = []; | ||
const objectsWithToString: any[] = []; | ||
|
||
// Allocate a bunch of objects with a unique structure. | ||
for (let i = 0; i < 1000000; i++) { | ||
objects.push({['foo_' + i]: i}); | ||
objectsWithToString.push({['foo_' + i]: i, toString: () => 'x'}); | ||
} | ||
const max = objects.length - 1; | ||
let i = 0; | ||
|
||
const benchmarkRefresh = createBenchmark('renderStringify'); | ||
const renderStringifyConcatTime = benchmarkRefresh('concat'); | ||
const renderStringifyConcatWithToStringTime = benchmarkRefresh('concat with toString'); | ||
const renderStringifyToStringTime = benchmarkRefresh('toString'); | ||
const renderStringifyToStringWithToStringTime = benchmarkRefresh('toString with toString'); | ||
const renderStringifyConstructorTime = benchmarkRefresh('constructor'); | ||
const renderStringifyConstructorWithToStringTime = benchmarkRefresh('constructor with toString'); | ||
const renderStringifyToStringMonoTime = benchmarkRefresh('toString mono'); | ||
const renderStringifyToStringWithToStringMonoTime = benchmarkRefresh('toString with toString mono'); | ||
|
||
// Important! This code is somewhat repetitive, but we can't move it out into something like | ||
// `benchmark(name, stringifyFn)`, because passing in the function as a parameter breaks inlining. | ||
|
||
// String concatenation | ||
while (renderStringifyConcatTime()) { | ||
renderStringifyConcat(objects[i]); | ||
i = i < max ? i + 1 : 0; | ||
} | ||
|
||
while (renderStringifyConcatWithToStringTime()) { | ||
renderStringifyConcat(objectsWithToString[i]); | ||
i = i < max ? i + 1 : 0; | ||
} | ||
///////////// | ||
|
||
// String() | ||
while (renderStringifyConstructorTime()) { | ||
renderStringifyConstructor(objects[i]); | ||
i = i < max ? i + 1 : 0; | ||
} | ||
|
||
while (renderStringifyConstructorWithToStringTime()) { | ||
renderStringifyConstructor(objectsWithToString[i]); | ||
i = i < max ? i + 1 : 0; | ||
} | ||
///////////// | ||
|
||
// toString | ||
while (renderStringifyToStringTime()) { | ||
renderStringifyToString(objects[i]); | ||
i = i < max ? i + 1 : 0; | ||
} | ||
|
||
while (renderStringifyToStringWithToStringTime()) { | ||
renderStringifyToString(objectsWithToString[i]); | ||
i = i < max ? i + 1 : 0; | ||
} | ||
///////////// | ||
|
||
// toString mono | ||
while (renderStringifyToStringMonoTime()) { | ||
renderStringifyToString(objects[0]); | ||
} | ||
|
||
while (renderStringifyToStringWithToStringMonoTime()) { | ||
renderStringifyToString(objectsWithToString[0]); | ||
} | ||
///////////// | ||
|
||
benchmarkRefresh.report(); |