1795848 - IRGenerator::stubName_: don't allocate these on the C++ heap

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P1)

Description

[Julian Seward [:jseward]]

Profiling of tcampbell's React benchmark [1] using DHAT shows that
about 20000 of a total of 195000 C++ heap allocated blocks are
clones of C string literals, created by JS_smprintf. This patch
removes those allocations entirely. This problem was alluded to
some time back in [2].

[1] https://github.com/mozilla-spidermonkey/matrix-react-bench

[2] https://phabricator.services.mozilla.com/D148035#inline-815309

Comment 1

[Julian Seward [:jseward]]

Some numbers, gathered from three different tools. Conclusion is that the
patch reduces the number of heap allocations by about 10% but does not change
anything much else. Also, there are no new leaks ("175,592 allocs, 175,592
frees") and (not visible from the text) no invalid frees.

BEFORE:

(memcheck)
total heap usage: 195,530 allocs, 195,530 frees, 151,660,810 bytes allocated
(dhat)
Total: 151,665,671 bytes in 195,515 blocks
At t-gmax: 35,224,963 bytes in 55,843 blocks
At t-end: 0 bytes in 0 blocks
Reads: 837,711,211 bytes
Writes: 375,911,282 bytes
(callgrind)
I refs: 3,395,035,440
D refs: 1,472,487,999 (868,043,742 rd + 604,444,257 wr)

AFTER:

(memcheck)
total heap usage: 175,592 allocs, 175,592 frees, 151,024,146 bytes allocated
(dhat)
Total: 151,018,434 bytes in 175,575 blocks
At t-gmax: 35,225,126 bytes in 55,845 blocks
At t-end: 0 bytes in 0 blocks
Reads: 837,856,566 bytes
Writes: 375,665,932 bytes
(callgrind)
I refs: 3,383,445,727
D refs: 1,467,651,851 (865,285,818 rd + 602,366,033 wr)

Comment 2

[Julian Seward [:jseward]]

Attachment: Bug 1795848 - IRGenerator::stubName_: don't allocate these on the C++ heap. r=iain.

Profiling of tcampbell's React benchmark [1] using DHAT shows that
about 20000 of a total of 195000 C++ heap allocated blocks are
clones of C string literals, created by JS_smprintf. This patch
removes those allocations entirely. This problem was alluded to
some time back in [2].

[1] https://github.com/mozilla-spidermonkey/matrix-react-bench

[2] https://phabricator.services.mozilla.com/D148035#inline-815309

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/004b8874a4e9
IRGenerator::stubName_: don't allocate these on the C++ heap.  r=iain.

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/004b8874a4e9

Comment 5

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #3)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/004b8874a4e9
IRGenerator::stubName_: don't allocate these on the C++ heap. r=iain.

== Change summary for alert #35719 (as of Fri, 21 Oct 2022 02:38:46 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
12%	imdb ContentfulSpeedIndex	windows10-64-shippable-qr	cold fission webrender	636.58 -> 561.17
9%	imdb fcp	windows10-64-shippable-qr	cold fission webrender	584.31 -> 533.83
8%	imdb fcp	windows10-64-shippable-qr	cold fission webrender	573.16 -> 525.42
8%	imdb PerceptualSpeedIndex	windows10-64-shippable-qr	cold fission webrender	645.05 -> 591.67
7%	imdb PerceptualSpeedIndex	windows10-64-shippable-qr	cold fission webrender	640.29 -> 598.50
...	...	...	...	...
3%	youtube loadtime	linux1804-64-shippable-qr	fission warm webrender	1,243.58 -> 1,209.88

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35719