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.

Sign up for GitHub

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

Jump to bottom

Still slow startup/shutdown for v8.x #14870

Closed
mhdawson opened this issue Aug 16, 2017 · 6 comments
Closed

Still slow startup/shutdown for v8.x #14870

mhdawson opened this issue Aug 16, 2017 · 6 comments
Labels
performance Issues and PRs related to the performance of Node.js.

Comments

@mhdawson
Copy link
Member

mhdawson commented Aug 16, 2017 •
edited
Loading

  • Version: v8.x
  • Platform: all
  • Subsystem: v8

I've been looking for startup performance to go back to normal after the changes for the hashseed problem.

I see that for master, but not for v8.x in the nightly graphs: https://benchmarking.nodejs.org/charts/start_stop.png

I looked at the jobs and it appears to be cloning the right repo and the raw results still show the slower startup time so I don't think its a problem in translation to the graphs:

Last nights run was:
https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-footprint-8.x/99/console

+ sh /home/iojs/build/workspace/benchmark-footprint-8.x/benchmarking/benchmarks/start_stop_time/calcStartup.sh
+ STARTUP_RESULT=262520
+ export BENCHID=2
+ /home/iojs/benchmarking/tools/postResults/postit.sh 6 2 262520
+ PUBLISH_RESULT=ok

which shows 262520 as opposed to 38920 for master.

I talked to Myles and the belief is that the fix was backported in v8.3 and v6.11.3 rc
@mhdawson
Copy link
Member Author

@MylesBorins @hashseed

@MylesBorins
Copy link
Contributor

@natorion

@mscdex mscdex added performance Issues and PRs related to the performance of Node.js. v8.x labels Aug 16, 2017
@MylesBorins
Copy link
Contributor

It looks like the re-enabling of snapshots didn't get backported to v8.x

I'm going to dig in a bit more this afternoon to figure out why

@MylesBorins MylesBorins mentioned this issue Aug 16, 2017
Merged
@addaleax addaleax mentioned this issue Aug 17, 2017
Closed
MylesBorins added a commit to MylesBorins/node that referenced this issue Aug 18, 2017
@MylesBorins
build: re-enable snapshots in v8.x
2d8d355 
When 5d5e31c was backported to v8.x as e2b306c it was missing the
configure changes from 8dce05f. This patch actually re-enables
snapshots!

Fixes: nodejs#14870

PR-URL: nodejs#14875
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@hashseed
Copy link
Member

Hehe. That might explain @bmeurer's confusion when he saw 8.3.0 start up slow and Turbofan running hot when my Node master branch worked just fine :D

@zertosh
Copy link
Contributor

zertosh commented Sep 1, 2017

Looks like #14875 barely missed v8.4.0, is it fair to say that this would make it into a v8.5.0?

MylesBorins added a commit that referenced this issue Sep 9, 2017
@MylesBorins
build: re-enable snapshots in v8.x
e12a9c5 
When 5d5e31c was backported to v8.x as e2b306c it was missing the
configure changes from 8dce05f. This patch actually re-enables
snapshots!

Fixes: #14870

PR-URL: #14875
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@bnoordhuis
Copy link
Member

Snapshots are enabled again in v8.5.0. I'll close out the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mscdex @bnoordhuis @MylesBorins @zertosh @hashseed @mhdawson