Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 75e22ab

Browse filesBrowse files
crisbetothePunderWoman
crisbeto
authored and
thePunderWoman
committed
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
1 parent 24c0897 commit 75e22ab
Copy full SHA for 75e22ab

File tree

4 files changed

+163
-1
lines changed
Filter options

4 files changed

+163
-1
lines changed

‎packages/core/src/render3/util/stringify_utils.ts

Copy file name to clipboardExpand all lines: packages/core/src/render3/util/stringify_utils.ts
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@
1010
* Used for stringify render output in Ivy.
1111
* Important! This function is very performance-sensitive and we should
1212
* be extra careful not to introduce megamorphic reads in it.
13+
* Check `core/test/render3/perf/render_stringify` for benchmarks and alternate implementations.
1314
*/
1415
export function renderStringify(value: any): string {
1516
if (typeof value === 'string') return value;
1617
if (value == null) return '';
17-
return '' + value;
18+
// Use `String` so that it invokes the `toString` method of the value. Note that this
19+
// appears to be faster than calling `value.toString` (see `render_stringify` benchmark).
20+
return String(value);
1821
}
1922

2023

‎packages/core/test/acceptance/text_spec.ts

Copy file name to clipboardExpand all lines: packages/core/test/acceptance/text_spec.ts
+42Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,46 @@ describe('text instructions', () => {
129129

130130
expect(div.innerHTML).toBe('function foo() { }');
131131
});
132+
133+
it('should stringify an object using its toString method', () => {
134+
class TestObject {
135+
toString() {
136+
return 'toString';
137+
}
138+
valueOf() {
139+
return 'valueOf';
140+
}
141+
}
142+
143+
@Component({template: '{{object}}'})
144+
class App {
145+
object = new TestObject();
146+
}
147+
148+
TestBed.configureTestingModule({declarations: [App]});
149+
const fixture = TestBed.createComponent(App);
150+
fixture.detectChanges();
151+
152+
expect(fixture.nativeElement.textContent).toBe('toString');
153+
});
154+
155+
it('should stringify a symbol', () => {
156+
// This test is only valid on browsers that support Symbol.
157+
if (typeof Symbol === 'undefined') {
158+
return;
159+
}
160+
161+
@Component({template: '{{symbol}}'})
162+
class App {
163+
symbol = Symbol('hello');
164+
}
165+
166+
TestBed.configureTestingModule({declarations: [App]});
167+
const fixture = TestBed.createComponent(App);
168+
fixture.detectChanges();
169+
170+
// Note that this uses `toContain`, because a polyfilled `Symbol` produces something like
171+
// `Symbol(hello)_p.sc8s398cplk`, whereas the native one is `Symbol(hello)`.
172+
expect(fixture.nativeElement.textContent).toContain('Symbol(hello)');
173+
});
132174
});

‎packages/core/test/render3/perf/BUILD.bazel

Copy file name to clipboardExpand all lines: packages/core/test/render3/perf/BUILD.bazel
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,16 @@ ng_benchmark(
255255
name = "view_destroy_hook",
256256
bundle = ":view_destroy_hook_lib",
257257
)
258+
259+
ng_rollup_bundle(
260+
name = "render_stringify_lib",
261+
entry_point = ":render_stringify/index.ts",
262+
deps = [
263+
":perf_lib",
264+
],
265+
)
266+
267+
ng_benchmark(
268+
name = "render_stringify",
269+
bundle = ":render_stringify_lib",
270+
)

‎packages/core/test/render3/perf/render_stringify/index.ts

Copy file name to clipboard
+104Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
import {createBenchmark} from '../micro_bench';
9+
10+
// These benchmarks compare various implementations of the `renderStringify` utility
11+
// which vary in subtle ways which end up having an effect on performance.
12+
13+
/** Uses string concatenation to convert a value into a string. */
14+
function renderStringifyConcat(value: any): string {
15+
if (typeof value === 'string') return value;
16+
if (value == null) return '';
17+
return '' + value;
18+
}
19+
20+
/** Uses `toString` to convert a value into a string. */
21+
function renderStringifyToString(value: any): string {
22+
if (typeof value === 'string') return value;
23+
if (value == null) return '';
24+
return value.toString();
25+
}
26+
27+
/** Uses the `String` constructor to convert a value into a string. */
28+
function renderStringifyConstructor(value: any): string {
29+
if (typeof value === 'string') return value;
30+
if (value == null) return '';
31+
return String(value);
32+
}
33+
34+
const objects: any[] = [];
35+
const objectsWithToString: any[] = [];
36+
37+
// Allocate a bunch of objects with a unique structure.
38+
for (let i = 0; i < 1000000; i++) {
39+
objects.push({['foo_' + i]: i});
40+
objectsWithToString.push({['foo_' + i]: i, toString: () => 'x'});
41+
}
42+
const max = objects.length - 1;
43+
let i = 0;
44+
45+
const benchmarkRefresh = createBenchmark('renderStringify');
46+
const renderStringifyConcatTime = benchmarkRefresh('concat');
47+
const renderStringifyConcatWithToStringTime = benchmarkRefresh('concat with toString');
48+
const renderStringifyToStringTime = benchmarkRefresh('toString');
49+
const renderStringifyToStringWithToStringTime = benchmarkRefresh('toString with toString');
50+
const renderStringifyConstructorTime = benchmarkRefresh('constructor');
51+
const renderStringifyConstructorWithToStringTime = benchmarkRefresh('constructor with toString');
52+
const renderStringifyToStringMonoTime = benchmarkRefresh('toString mono');
53+
const renderStringifyToStringWithToStringMonoTime = benchmarkRefresh('toString with toString mono');
54+
55+
// Important! This code is somewhat repetitive, but we can't move it out into something like
56+
// `benchmark(name, stringifyFn)`, because passing in the function as a parameter breaks inlining.
57+
58+
// String concatenation
59+
while (renderStringifyConcatTime()) {
60+
renderStringifyConcat(objects[i]);
61+
i = i < max ? i + 1 : 0;
62+
}
63+
64+
while (renderStringifyConcatWithToStringTime()) {
65+
renderStringifyConcat(objectsWithToString[i]);
66+
i = i < max ? i + 1 : 0;
67+
}
68+
/////////////
69+
70+
// String()
71+
while (renderStringifyConstructorTime()) {
72+
renderStringifyConstructor(objects[i]);
73+
i = i < max ? i + 1 : 0;
74+
}
75+
76+
while (renderStringifyConstructorWithToStringTime()) {
77+
renderStringifyConstructor(objectsWithToString[i]);
78+
i = i < max ? i + 1 : 0;
79+
}
80+
/////////////
81+
82+
// toString
83+
while (renderStringifyToStringTime()) {
84+
renderStringifyToString(objects[i]);
85+
i = i < max ? i + 1 : 0;
86+
}
87+
88+
while (renderStringifyToStringWithToStringTime()) {
89+
renderStringifyToString(objectsWithToString[i]);
90+
i = i < max ? i + 1 : 0;
91+
}
92+
/////////////
93+
94+
// toString mono
95+
while (renderStringifyToStringMonoTime()) {
96+
renderStringifyToString(objects[0]);
97+
}
98+
99+
while (renderStringifyToStringWithToStringMonoTime()) {
100+
renderStringifyToString(objectsWithToString[0]);
101+
}
102+
/////////////
103+
104+
benchmarkRefresh.report();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.