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
This commit is contained in:
Kristiyan Kostadinov 2020-11-28 12:01:57 +01:00 committed by Jessica Janiuk
parent 1de59d2818
commit 11cd37fae3
4 changed files with 163 additions and 1 deletions

View File

@ -10,11 +10,14 @@
* Used for stringify render output in Ivy. * Used for stringify render output in Ivy.
* Important! This function is very performance-sensitive and we should * Important! This function is very performance-sensitive and we should
* be extra careful not to introduce megamorphic reads in it. * be extra careful not to introduce megamorphic reads in it.
* Check `core/test/render3/perf/render_stringify` for benchmarks and alternate implementations.
*/ */
export function renderStringify(value: any): string { export function renderStringify(value: any): string {
if (typeof value === 'string') return value; if (typeof value === 'string') return value;
if (value == null) return ''; if (value == null) return '';
return '' + value; // Use `String` so that it invokes the `toString` method of the value. Note that this
// appears to be faster than calling `value.toString` (see `render_stringify` benchmark).
return String(value);
} }

View File

@ -129,4 +129,46 @@ describe('text instructions', () => {
expect(div.innerHTML).toBe('function foo() { }'); expect(div.innerHTML).toBe('function foo() { }');
}); });
it('should stringify an object using its toString method', () => {
class TestObject {
toString() {
return 'toString';
}
valueOf() {
return 'valueOf';
}
}
@Component({template: '{{object}}'})
class App {
object = new TestObject();
}
TestBed.configureTestingModule({declarations: [App]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('toString');
});
it('should stringify a symbol', () => {
// This test is only valid on browsers that support Symbol.
if (typeof Symbol === 'undefined') {
return;
}
@Component({template: '{{symbol}}'})
class App {
symbol = Symbol('hello');
}
TestBed.configureTestingModule({declarations: [App]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
// Note that this uses `toContain`, because a polyfilled `Symbol` produces something like
// `Symbol(hello)_p.sc8s398cplk`, whereas the native one is `Symbol(hello)`.
expect(fixture.nativeElement.textContent).toContain('Symbol(hello)');
});
}); });

View File

@ -255,3 +255,16 @@ ng_benchmark(
name = "view_destroy_hook", name = "view_destroy_hook",
bundle = ":view_destroy_hook_lib", bundle = ":view_destroy_hook_lib",
) )
ng_rollup_bundle(
name = "render_stringify_lib",
entry_point = ":render_stringify/index.ts",
deps = [
":perf_lib",
],
)
ng_benchmark(
name = "render_stringify",
bundle = ":render_stringify_lib",
)

View File

@ -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();