-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Performance issue in NodeJS >= 10.0.0 #27667
Comments
Looks like, it's an issue with |
Can you run (The reason I ask you to do it is that it might be something local to your machine. That should hopefully show up in the profile log.) edit: if it freezes, you might want to ^C it after a few seconds. That should still result in a usable log file. |
NodeJS version 8.16.0:
NodeJS version 10.0.0:
|
I don't think that it has to do something with AJV library since the extracted validation function in the example script is pure and simple JavaScript code, which even does not have any loops in it. Further also the testing data, an array of 1000 elements that however in fact only consists of two different elements (one failing and one succeeding) cloned multiple times, should be no real problem. |
Thanks, I see roughly the same thing: exponential behavior in the (edit: tier 2) JS compiler.
I'm reasonably sure it was introduced by v8/v8@947101c8734 so cc @bmeurer. That also explains why Node.js v8.x is unaffected because that change was introduced in V8 6.3 and Node.js v8.x ships 6.2. edit: forgot to mention, V8 ToT seems to be affected as well. |
Good catch. I filed v8:9250 upstream. |
Consider floating this patch in Node 10 / 12. |
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in nodejs#27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: nodejs#27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: nodejs#27667
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in #27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: #27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: #27667 PR-URL: #27729 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in nodejs#27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: nodejs#27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: nodejs#27667 PR-URL: nodejs#27729 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in nodejs#27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: nodejs#27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: nodejs#27667 PR-URL: nodejs#27729 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in nodejs#27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: nodejs#27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: nodejs#27667 PR-URL: nodejs#27729 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in nodejs#27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: nodejs#27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: nodejs#27667 PR-URL: nodejs#27729 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: Remove recursion from NeedsCheckHeapObject. We use the predicate NeedsCheckHeapObject in the compiler frontend to determine whether we can skip introducing CheckHeapObject nodes. But this predicate would also walk up the graph in case of Phis, which can result in really long compilation times (on the main thread). In the report in nodejs#27667, the compiler frontend alone took around 4-5mins of main thread time for a single function. With this patch the time goes down to 4-5ms. Bug: v8:9250 Refs: nodejs#27667 Change-Id: I231eb780ff04f949fa1669714f9af6ebfbcade05 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1612897 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#61503} Fixes: nodejs#27667 Backport-PR-URL: nodejs#28005 PR-URL: nodejs#27729 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Not sure if this is the right place but I may have found a possible performance issue in NodeJS JavaScript execution.
While execution of this example script on a NodeJS runtime <10.0.0 runs smoothly, it freezes on NodeJS runtimes >= 10.0.0 for an indefinite amount of time.
I tested the script for all latest major releases of NodeJS and got the following results:
I tested the script on different machines (including windows hosts), always getting the same result.
Due to the facts that the example-script is a pure JavaScript file with no dependencies on any NodeJS or external modules, and that the issue occurred first in NodeJS version 10.0.0 I assume that it has to do something with the upgrade of V8 Runtime from 6.2 to 6.6, which was listed in the changelog ov NodeJS version 10.0.0.
NB: I discovered this issue after upgrading a service to newest NodeJS runtime. The service is responsible for validating arbitrary Data against a defined JSONSchema. Validation is implemented using an external library called AJV. After the update, the service started to freeze sporadically during validation process. The provided example script therefore contains an example of a validation function generated by AJV library and some sample data (array of 1000 elements) for which validation will freeze after some time (on my machine, depending on NodeJs version at element no ~594 or no ~610).
The text was updated successfully, but these errors were encountered: