Closed Bug 1795848 Opened 4 months ago Closed 4 months ago

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

Categories

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

task

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox108 --- fixed

People

(Reporter: jseward, Assigned: jseward)

References

(Blocks 3 open bugs)

Details

(Keywords: perf-alert)

Attachments

(1 file)

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

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)

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

Severity: -- → N/A
Priority: -- → P1
Blocks: 1796093
Pushed by jseward@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/004b8874a4e9
IRGenerator::stubName_: don't allocate these on the C++ heap.  r=iain.
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch
Blocks: sm-jits

(In reply to Pulsebot from comment #3)

Pushed by jseward@mozilla.com:
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

Blocks: 1801189
You need to log in before you can comment on or make changes to this bug.