2023167 - Add Gecko Profiler support for RISCV64 and LoongArch64

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P3)

Description

[Rong Bao [:csmantle]]

Starting from Bug 1403421, we've already enjoyed Gecko Profiler support on MIPS64. This bug tracks my efforts to implement the same for RV64 and LA64, two Tier-3 platforms. These architectures are gaining adoption recently, and having a working in-browser profiler for them would greatly boost user experience.

Comment 1

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 1: Move field initializers of {Base}Profiler's Registers class to declaration. r=#profiler-reviewers!

Comment 2

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 3: Port Gecko Profiler to loongarch64-linux. r=#profiler-reviewers!

Comment 3

[Rong Bao [:csmantle]]

Part 1 and Part 2 are tested by kf <[e-mail]> on Loongson XA61200 (3A6000) by capturing a trace using F12 - Performance on a Speedometer Main run, after working around Bug 2023180.

Captured profile: https://share.firefox.dev/4sEvD2J

Comment 4

[Rong Bao [:csmantle]]

Attachment: LoongArch64 demo screenshot 1

Comment 5

[Rong Bao [:csmantle]]

Comment on attachment 9552985 [details]
LoongArch64 demo screenshot 1

Courtesy of kf <[e-mail]>.

Comment 6

[Rong Bao [:csmantle]]

Attachment: LoongArch64 demo screenshot 2

Courtesy of kf <[e-mail]>.

Comment 7

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 4: Port Gecko Profiler to riscv64-linux. r=#profiler-reviewers!

Comment 8

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 4: (Drive-by) Fix typo: "rEDZONE" -> "REDZONE". r=#profiler-reviewers!

Comment 9

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 5: (Drive-by) Lower constant MOZ_ASSERTs to static_asserts. r=#profiler-reviewers!

Comment 10

[Rong Bao [:csmantle]]

Attachment: RV64 demo screenshot 1

Taken by myself on Milk-V Jupiter (SpacemiT M1).

Profile: https://share.firefox.dev/40AJ73P

Comment 11

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 4: Track more registers on LA and RV for CFA. r=#profiler-reviewers!

• t0 on RV and $t0 on LA are used by glibc.
• a1 on RV and $a1 on LA are used by libffi.

Link:
https://elixir.bootlin.com/glibc/glibc-2.43/source/sysdeps/unix/sysv/linux/loongarch/setcontext.S#L49
Link:
https://elixir.bootlin.com/glibc/glibc-2.43/source/sysdeps/unix/sysv/linux/riscv/setcontext.S#L46
Link:
https://github.com/libffi/libffi/blob/10056e7e6a0d40d2a21af63484b99f08898dde9e/src/loongarch64/sysv.S#L92
Link:
https://github.com/libffi/libffi/blob/10056e7e6a0d40d2a21af63484b99f08898dde9e/src/riscv/sysv.S#L90

Comment 12

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 6: Implement RV64 and LA64 frame chasing fallback. r=#profiler-reviewers!

Comment 13

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 8: Accept def_cfa_register as the first CFI opcode. r=#profiler-reviewers!

This change is learned from Breakpad.

Link:
https://chromium.googlesource.com/breakpad/breakpad/+/f78839c15730533fc433c50ea83a3afea835c245%5E%21/#F12

Comment 14

[Rong Bao [:csmantle]]

It seems that this stack has become too large. I'll split it into other bugs that allow fine-grained review.

Comment 15

[Phabricator Automation]

Comment on attachment 9570956 [details]
Bug 2023167 - Part 8: Accept def_cfa_register as the first CFI opcode. r=#profiler-reviewers!

Revision D294884 was moved to bug 2033272. Setting attachment 9570956 [details] to obsolete.

Comment 16

[Phabricator Automation]

Comment on attachment 9552683 [details]
Bug 2023167 - Part 1: Move field initializers of {Base}Profiler's Registers class to declaration. r=#profiler-reviewers!

Revision D287578 was moved to bug 2033273. Setting attachment 9552683 [details] to obsolete.

Comment 17

[Phabricator Automation]

Comment on attachment 9552992 [details]
Bug 2023167 - Part 4: (Drive-by) Fix typo: "rEDZONE" -> "REDZONE". r=#profiler-reviewers!

Revision D287781 was moved to bug 2033273. Setting attachment 9552992 [details] to obsolete.

Comment 18

[Phabricator Automation]

Comment on attachment 9553655 [details]
Bug 2023167 - Part 5: (Drive-by) Lower constant MOZ_ASSERTs to static_asserts. r=#profiler-reviewers!

Revision D288164 was moved to bug 2033273. Setting attachment 9553655 [details] to obsolete.

Comment 19

[Nazım Can Altınova [:canova]]

Hi :csmantle!

We couldn't review your patches yet, sorry! I have a few questions before I get into the reviewing the important parts of the code changes:

1. Are all of the patches ready for review? Does the native stack walking as well as the rest of the profiler work in RISCV64 and LoongArch64?
2. How can we test these changes locally? As I'm not really familiar with the architecture, I kind of feel like I'm in the dark while reviewing the patches. It would be great if you can provide a manual testing STR. Since we don't have machines for these platforms, it's pretty difficult to gauge. For example, I assume I can download QEMU and then install a linux distro that distributes RISCV64 and LoongArch64 binaries. And then I can try to compile it to see if it works locally. Could you provide us more concrete testing steps?

Comment 20

[Rong Bao [:csmantle]]

Hi :canova,

(In reply to Nazım Can Altınova [:canova][:canaltinova on phabricator] from comment #19)

1. Are all of the patches ready for review? Does the native stack walking as well as the rest of the profiler work in RISCV64 and LoongArch64?

The current state is that the code compiles, can capture and upload profiles, and the basic browsing works. I'm still debugging the missing frames (as mentioned in the Matrix thread), and it seems that symbol files for these archs are lacking.

The primary goal of this bug is to make Gecko Profiler at least as usable as on MIPS64 (tier-3, same as RISC-V and LoongArch), where it already has some support.

2. How can we test these changes locally?
   For example, I assume I can download QEMU and then install a linux distro that distributes RISCV64 and LoongArch64 binaries. And then I can try to compile it to see if it works locally.

This is plausible but would be very slow. I'll ask my friends at Anthon OS Community to see if they could help. They have significant support efforts into and user groups of these archs. I actually have a version of Fx with this stack landed [^0] there, and the profiles posted in the comments above were collected on that version.

Could you provide us more concrete testing steps?

Honestly my current testing steps are quite casual:

• Go to https://browserben.ch/speedometer/v3.1/.
• Launch the profiler, start capturing with "Firefox" preset.
• Start Speedometer, and let it run for a couple of seconds.
• Stop the capture and observe.

So I'd be delighted to know a more systematic approach :)

Comment 21

[jeffbai]

(In reply to Nazım Can Altınova [:canova][:canaltinova on phabricator] from comment #19)

Hi :csmantle!

We couldn't review your patches yet, sorry! I have a few questions before I get into the reviewing the important parts of the code changes:

1. Are all of the patches ready for review? Does the native stack walking as well as the rest of the profiler work in RISCV64 and LoongArch64?
2. How can we test these changes locally? As I'm not really familiar with the architecture, I kind of feel like I'm in the dark while reviewing the patches. It would be great if you can provide a manual testing STR. Since we don't have machines for these platforms, it's pretty difficult to gauge. For example, I assume I can download QEMU and then install a linux distro that distributes RISCV64 and LoongArch64 binaries. And then I can try to compile it to see if it works locally. Could you provide us more concrete testing steps?

Hi Nazım,

Rong notified me about your question about potential hardware access. While I can't help with RISC-V, I could potentially help you connect with SSH access or a physical device (depending on where you are located). Happy to help if you can email me via the contact below:

https://loongfans.cn/en/intro#i-m-sold-where-can-i-buy-one

Comment 22

[Nazım Can Altınova [:canova]]

(In reply to Rong Bao [:csmantle] from comment #20)

Honestly my current testing steps are quite casual:

• Go to https://browserben.ch/speedometer/v3.1/.
• Launch the profiler, start capturing with "Firefox" preset.
• Start Speedometer, and let it run for a couple of seconds.
• Stop the capture and observe.

So I'd be delighted to know a more systematic approach :)

That sounds good. Another thing worth checking is to run all the profiler tests in these archs to see they all pass. For example, our tests mostly live in tools/profiler/tests and devtools/client/performance-new/test. So running the tests in these directories will most likely show you if there is something broken. We have some native stack collection tests there as well, but they don't check exactly if "all" of the frames look correct. Still it's something worth checking.

(In reply to jeffbai from comment #21)

Hi Nazım,

Rong notified me about your question about potential hardware access. While I can't help with RISC-V, I could potentially help you connect with SSH access or a physical device (depending on where you are located). Happy to help if you can email me via the contact below:

https://loongfans.cn/en/intro#i-m-sold-where-can-i-buy-one

Hi, thanks for the offer! It would be great to have an SSH access for me to check at least. Just reached out to you via email.

Comment 23

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 5: Track WASM tail call scratch registers on LA and RV. r=#profiler-reviewers!

• WasmTailCallFPScratchReg: t3 on RV and $t3 on LA.
• WasmTailCallRAScratchReg: ra on RV and $ra on LA.

Comment 24

[Rong Bao [:csmantle]]

(In reply to Nazım Can Altınova [:canova][:canaltinova on phabricator] from comment #22)

(In reply to Rong Bao [:csmantle] from comment #20)
Another thing worth checking is to run all the profiler tests in these archs to see they all pass. For example, our tests mostly live in tools/profiler/tests and devtools/client/performance-new/test. So running the tests in these directories will most likely show you if there is something broken.

These tests are super helpful!

I think we're almost there now. I ironed out some glitches in frame chasing code. And it seems that both LoongArch and RISC-V have stacks larger than 160KB [^0], which is probably why the libc frames are not captured.

I'll update the diffs once more testing is done, but here's a profile for LA64 I captured yesterday: https://share.firefox.dev/4cUc8gj. Once done, I'll re-request review for the updated diffs.

CC-ing Markus here :)

Comment 25

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 7: Add support for DwarfCFIToModule::UndefinedRule. r=#profiler-reviewers!

This adds support for undefining calculation rule for a given (address,
reg) pair.

Comment 26

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 8: Increase stack snapshot size on {loongarch64,riscv64}-linux. r=#profiler-reviewers!

The stack size needed on these platforms are much larger than that on
x64, so allocating more space for it.

Comment 27

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 9: Move lul::StackImage out of stack on {loongarch64,riscv64}-linux. r=#profiler-reviewers!

The purpose of this patch is to work around huge frames on these
platforms. Clang has little support for -fstack-clash-protection on
LoongArch, while on RV64 this flag is not enabled in build config by
default.

For async sampling, we store the UniquePtr to a heap-allocated
lul::StackImage in SamplerThread, and pass the pointer around when doing
LUL backtraces.

For sync sampling, we allocate space for lul::StackImage in the thread
local storage, and pass the pointer into LUL backtracer.

This patch is organized so that only {loongarch64,riscv64}-linux are
affected, while other architectures should not be impacted.

Comment 28

[Rong Bao [:csmantle]]

This profile is captured with this new stack applied on LoongArch64: https://share.firefox.dev/4cJXSYR

Comment 29

[Rong Bao [:csmantle]]

(In reply to Rong Bao [:csmantle] from comment #27)

Created attachment 9575795 [details]
Bug 2023167 - Part 8: Move lul::StackImage out of stack on {loongarch64,riscv64}-linux. r=#profiler-reviewers!

The purpose of this patch is to work around huge frames on these
platforms. Clang has little support for -fstack-clash-protection on
LoongArch, while on RV64 this flag is not enabled in build config by
default. [...]

I filed https://github.com/llvm/llvm-project/pull/195595 for LLVM, but I guess this workaround would have to stay for a while...

Comment 30

[Rong Bao [:csmantle]]

FYI: https://github.com/llvm/llvm-project/pull/195595 has been merged for Clang 23.

Comment 31

[Rong Bao [:csmantle]]

Gentle ping.

Comment 32

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 1: Reorganize EnterJITRegs on loong64 to allow easier frame chasing across JIT/native boundary. r=#spidermonkey-reviewers!

This moves the saved $fp to $fp+0 and saved $ra to $fp+8, which looks
the same as the JS ABI frame layout. This can aid LUL frame chasing that
has just walked the JS frames. The risk should be minimal since
EnterJITRegs's layout should be opaque to deeper frames after all.

Comment 33

[Rong Bao [:csmantle]]

Attachment: Bug 2023167 - Part 2: Reorganize EnterJITRegs on riscv64 to allow easier frame chasing across JIT/native boundary. r=#spidermonkey-reviewers!

Ditto for riscv64; see Part 1.

Comment 34

[Rong Bao [:csmantle]]

Profile https://share.firefox.dev/4vIssbx is captured with a build containing the latest stack. I think the majority of stacks are reasonable (apart from missing symbols), both JS and native ones.

Comment 35

[Rong Bao [:csmantle]]

Good news! LLVM 23.1.0 is now released. This version contains my implementation of -fstack-clash-protection on LoongArch and my fix for this feature on RISC-V.

This means that, with an up-to-date toolchain, the Part 9 workaround would no longer be required.

I would do more testing on these platforms and drop Part 9 once confirmed.

Comment 36

[Rong Bao [:csmantle]]

So, after some experimentation:

1. The increase in stack snapshot size in Part 8 has no observable effect on the Speedometer 3.1 profile.

   • Without Part 8 (160 KiB): https://share.firefox.dev/3Sz4HFe
   • With Part 8 (256 KiB): https://share.firefox.dev/4zHoHpT

Both profiles show a similar stack shape with no missing frames.

2. The originally observed SIGSEGV on loongarch64 and riscv64 was caused by overflowing the 256 KiB per-thread stack size limit [^0]. So, while SCP helps, since sampling happens on a non-main thread, it couldn't prevent the crash.

In fact, for Speedometer 3.1, abandoning both Part 8 and 9 can also produce a successful and complete capture: https://share.firefox.dev/4qHJ56b

Comment 37

[Rong Bao [:csmantle]]

(In reply to Rong Bao [:csmantle] from comment #36)

Both profiles show a similar stack shape with no missing frames.

Oh, it seems that I missed something near the bottom of the samples display.

There is a tiny number of samples that have truncated frames:

• 1 with Part 8 reverted and Part 9 present.
• 5 with both Part 8 and Part 9 reverted.

All of these have this stack shape:

[...]
Run [/home/csmantle/workspace/mozilla-firefox-dev/obj-full/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc]
XRE_InitChildProcess [/home/csmantle/workspace/mozilla-firefox-dev/toolkit/xre/nsEmbedFunctions.cpp]
XRE_InitChildProcess
main [/home/csmantle/workspace/mozilla-firefox-dev/browser/app/nsBrowserApp.cpp]
(root)

So I guess snapshot size does play some part here, but not very significantly.

As shown by (currently abandoned) Part 9, accommodating this would require working around the per-thread stack size limit, and thus introduce extra complexity and overhead.

Markus, would you like to share some thoughts? I'm curious about why this isn't biting on Tier-1 platforms like x64. And just like you mentioned in [^0], if stack truncation could really happen on such platforms, it would be better to improve Part 9 to be used on all platforms with less per-thread memory overhead.

Comment 38

[Rong Bao [:csmantle]]

Thanks for your review comments! I've replied in Phabricator.

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/02e58c546f0a
https://hg.mozilla.org/integration/autoland/rev/cd275d1d76a2
Part 1: Reorganize EnterJITRegs on loong64 to allow easier frame chasing across JIT/native boundary. r=spidermonkey-reviewers,jandem
https://github.com/mozilla-firefox/firefox/commit/ff3ec66db0f6
https://hg.mozilla.org/integration/autoland/rev/9cdf0d732b31
Part 2: Reorganize EnterJITRegs on riscv64 to allow easier frame chasing across JIT/native boundary. r=spidermonkey-reviewers,jandem
https://github.com/mozilla-firefox/firefox/commit/b267ab5288be
https://hg.mozilla.org/integration/autoland/rev/9078e0704d50
Part 3: Port Gecko Profiler to loongarch64-linux. r=profiler-reviewers,mstange
https://github.com/mozilla-firefox/firefox/commit/5d458763323d
https://hg.mozilla.org/integration/autoland/rev/42b7f67542d2
Part 4: Port Gecko Profiler to riscv64-linux. r=profiler-reviewers,mstange
https://github.com/mozilla-firefox/firefox/commit/a1160356bba7
https://hg.mozilla.org/integration/autoland/rev/144527eaa51b
Part 5: Track WASM tail call scratch registers on LA and RV. r=profiler-reviewers,mstange
https://github.com/mozilla-firefox/firefox/commit/6699b0755601
https://hg.mozilla.org/integration/autoland/rev/6c2cca3ee59e
Part 6: Implement RV64 and LA64 frame chasing fallback. r=profiler-reviewers,mstange
https://github.com/mozilla-firefox/firefox/commit/c3760a0afbc2
https://hg.mozilla.org/integration/autoland/rev/79c0376ef206
Part 7: Add support for DwarfCFIToModule::UndefinedRule. r=mstange

Comment 40

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/cd275d1d76a2
https://hg.mozilla.org/mozilla-central/rev/9cdf0d732b31
https://hg.mozilla.org/mozilla-central/rev/9078e0704d50
https://hg.mozilla.org/mozilla-central/rev/42b7f67542d2
https://hg.mozilla.org/mozilla-central/rev/144527eaa51b
https://hg.mozilla.org/mozilla-central/rev/6c2cca3ee59e
https://hg.mozilla.org/mozilla-central/rev/79c0376ef206