You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
canova opened this issue
Oct 25, 2023
· 2 comments
Labels
c++Issues and PRs that require attention from people who are familiar with C++.cryptoIssues and PRs related to the crypto subsystem.opensslIssues and PRs related to the OpenSSL dependency.
We were investigating a performance regression for the profiler-server repository that we have for the Firefox Profiler. The latest staging branch also includes node v16 to v18 upgrade. After some profiling we realized that the regression is caused by createPublicKey and createPrivateKey APIs starting with, and more specifically the init functions called inside these APIs.
Performance measurements
It looks like you've recently added a benchmark for these APIs, that helped me a lot to identify the issue. It also made me suspicious that maybe you are already aware of this issue, but I couldn't find a filed bug for it.
Here's the performance measurements of this benchmark on node v16 (76f9a7d) (this benchmark file doesn't exist in node v16 but I copy and pasted that file manually):
As you can see here there is a huge difference between node v16 and node v18+
Preliminary performance profiles
I have captured some profiles both with Chrome's DevTool and also with samply
Profiles from Chrome DevTool with our loadtests: v16 - v18
These profiles are captured from the profiler-server loadtests, but you can clearly see that createPublicKey and createPrivateKey are now taking a lot more time.
These profiles were the ones that made us aware that these APIs might have regression. Then we went ahead and looked for a more targeted test which we found create-keyobject.js benchmark.
Performance profiles with C++ stacks and STR
So after finding out that these APIs are the culprit I profiled the create-keyobject.js benchmarks with samply. You can see the profiles here:
As you can see the duration of the test goes from 46ms to 265ms. And it looks like most of the time is spent on node::crypto::KeyObjectHandle::Init method (it had 8 samples before and now it has 248 samples). So I think we are calling this a lot, or this has started to take a lot more time. I don't know this codebase, so it would be great if a node crypto experts take a look at it.
STR for profiling yourself:
Install samply by running cargo install samply. (it requires rust)
Check out to the branch you want to test and build node.
If you don't have the benchmark/crypto/create-keyobject.js on that branch, manually add the file.
Run the benchmark tests with samply using this command: samply record ./node benchmark/crypto/create-keyobject.js
It will open up the Firefox Profiler UI in the browser after it's done.
Repeat it for the other branch.
Hope this helps identifying and fixing the problem!
I'm on macOS 14.0 with M1 Max CPU, but we observed the same issue on Linux as well, I believe it's platform agnostic.
The text was updated successfully, but these errors were encountered:
Yes, it is. I'll take the liberty of closing this because it's a) a known issue, and b) not under our control (3p dep), but thanks for the excellent bug report.
c++Issues and PRs that require attention from people who are familiar with C++.cryptoIssues and PRs related to the crypto subsystem.opensslIssues and PRs related to the OpenSSL dependency.
We were investigating a performance regression for the profiler-server repository that we have for the Firefox Profiler. The latest staging branch also includes node v16 to v18 upgrade. After some profiling we realized that the regression is caused by
createPublicKey
andcreatePrivateKey
APIs starting with, and more specifically theinit
functions called inside these APIs.Performance measurements
It looks like you've recently added a benchmark for these APIs, that helped me a lot to identify the issue. It also made me suspicious that maybe you are already aware of this issue, but I couldn't find a filed bug for it.
Here's the performance measurements of this benchmark on node v16 (76f9a7d) (this benchmark file doesn't exist in node v16 but I copy and pasted that file manually):
And here's the same benchmark measurements on latest main branch (0fb5123):
As you can see here there is a huge difference between node v16 and node v18+
Preliminary performance profiles
I have captured some profiles both with Chrome's DevTool and also with samply
Profiles from Chrome DevTool with our loadtests:
v16 - v18
These profiles are captured from the profiler-server loadtests, but you can clearly see that
createPublicKey
andcreatePrivateKey
are now taking a lot more time.These profiles were the ones that made us aware that these APIs might have regression. Then we went ahead and looked for a more targeted test which we found
create-keyobject.js
benchmark.Performance profiles with C++ stacks and STR
So after finding out that these APIs are the culprit I profiled the
create-keyobject.js
benchmarks with samply. You can see the profiles here:v16.x branch - main branch
As you can see the duration of the test goes from 46ms to 265ms. And it looks like most of the time is spent on
node::crypto::KeyObjectHandle::Init
method (it had 8 samples before and now it has 248 samples). So I think we are calling this a lot, or this has started to take a lot more time. I don't know this codebase, so it would be great if a node crypto experts take a look at it.STR for profiling yourself:
cargo install samply
. (it requires rust)benchmark/crypto/create-keyobject.js
on that branch, manually add the file.samply record ./node benchmark/crypto/create-keyobject.js
Hope this helps identifying and fixing the problem!
I'm on macOS 14.0 with M1 Max CPU, but we observed the same issue on Linux as well, I believe it's platform agnostic.
The text was updated successfully, but these errors were encountered: