Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf are deprecated on node 8 or above? #407

Closed
RafaelGSS opened this issue Jul 8, 2020 · 2 comments
Closed

perf are deprecated on node 8 or above? #407

RafaelGSS opened this issue Jul 8, 2020 · 2 comments

Comments

@RafaelGSS
Copy link
Member

Hello everyone.

I currently use perf to get stackframes of my application to improve the performance of it.
But in this issue: #148

  1. perf(1) support is now deprecated in V8 and will not be supported starting in Node 8 -- which effectively means we’re losing the ability to profile JS stacks.

Is perf already deprecated? How I can still get the stackframes and generates graphs of CPU profiler without perf?

Thanks!

@mmarchini
Copy link
Contributor

tl;dr perf(1) was never supported by V8, but it works. There are some gaps on Node.js v8.x, but that's an EOL version anyway, perf(1) works fine on all current release lines by combining --perf-basic-prof flag with --interpreted-frames-native-stack. See also this guide

Note that the issue is from early 2018, a lot has changes since then, but let me try to give some context.

Let's start with the state of affairs pre-Node.js v8.x. perf(1) was not really deprecated by V8 because it was never officially supported in the first place. "deprecated" and "officially supported" here means the flags necessary for perf(1) to work (--perf-basic-prof* or --perf-prof*) are not a first-class concern of V8: there are some tests for those flags and if those tests fail whoever sent a patch might try to fix it if it is an easy fix, but if the flags functionality gets in the way it might get removed. That's unlikely to happen though, the code is isolated enough that it doesn't get in the way of other features. The flag also relies on internals that V8 uses for other slightly more supported options (because they use options during development).

Prior to Node.js v8.x, all JavaScript code ran by V8 was Just-in-Time (JIT) compiled, which means all JS code would become native code. perf(1) is a low level profiler, it works best with native code following the processors call stack convention. So the V8/perf(1) compatibility up to v8.x was more lucky than design: being a JIT compiler, V8 generated and ran JS code in call stacks that perf(1) knew how to unwind, and the --perf-basic-prof* flags generate a map file that perf(1) uses for mapping functions on the stack which are not part of the executing binary.

The old compiler pipeline V8 used back then (Crankshaft) had several performance pitfalls, was hard to extend for new ECMAScript features, and as mentioned before was JIT-only, which prevented it from running on restricted environments where executing JIT code is disallowed (iOS has this restriction). Starting with Node.js v8.x, V8 introduced a new compiler pipeline (TurboFan + Ignition) which is a interpreter + JIT compiler: all JS code is first loaded and executed by the interpreter, which will collect execution information so that V8 can optimize the code later and compile it using the JIT compiler. This new pipeline came with a huge performance improvement and is a lot simpler to extend than the old one.

How does this new pipeline affects perf(1)? All JIT-compiled code still works perfrectly with perf(1) and --perf-basic-prof*, as explained above, but now we also have interpreted code running on the stack. perf(1) doesn't play well with interpreters as it relies on assumptions from native code execution, so for any JavaScript function running on the interpreter users will see an interpreter frame instead of a JS function frame. That's just a side effect of the interpreter + JIT-compiled design, and not a bug: perf(1) didn't break on Node.js v8.x, it still does exactly what it is supposed to, but the information it outputs for the user can be less relevant in this context.

V8's recommendation always was to use the V8 CPU Profiler to profile JavaScript code, which works when JS is the only thing running and the bottleneck is on JS. Therefore, it should work for most users. But there's a subset of users who still need perf(1) (heavy native module users or users who hit a regression on Node.js/V8/libuv). V8 CPU Profiler also had several performance issues back in Node.js v8.x (as shown here), but most of these issues are fixed now on Node.js v12.x.

We worked with V8 on a workaround for the incompatibility between perf(1) and interpreters, which resulted in the --interpreted-frames-native-stack flag for V8. With that flag, perf(1) can properly map interpreter frames to their JavaScript function names. In theory the flag will have a small memory and CPU overhead, in practice it will be so small on most cases that it won't be noticeable. This flag is a bit more intrusive on V8 code though, so it could get removed in a future release.

We also worked with V8 to introduce a new C++ API which allows embedders (such as Node.js or native modules) to implement the same behavior from --perf-basic-prof* on our side. The linux-perf uses this API to provide a on-demand generation of the map file used by perf(1) (instead of using the flag, which would write to the file indefinitely and eventually filling the entire disk or cause disk i/o operations to get significantly slower). The API also means we could re-implement --perf-basic-prof* flags on Node.js if V8 decides to remove it, but it's unlikely that they would remove it anyway. V8 APIs also follow V8 deprecation policy, so the API won't get removed without notice (it takes at least two or three V8 majors to remove an API). Unfortunately there's no way to implement --interpreted-frames-native-stack with any APIs, and such an API is unlikely to be released in the future.

So yes, technically it is deprecated, but everything is working on all current release lines of Node.js (v10.x, v12.x v14.x).

@RafaelGSS
Copy link
Member Author

Well, thanks so much for the reply. All the doubts are done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants