Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jul 8, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

bencmark, dgram

Description of change

Exit process so benchmark can continue.

process.exit() was removed previously at 83432bf#diff-1cc47d7eb19f32771e7d3f6d813eb92cL242

This change adds it back, but only in this benchmark and not in benchmark/common.js.

/cc @mscdex

Exit process so benchmark can continue.
@Trott Trott added dgram Issues and PRs related to the dgram subsystem / UDP. benchmark Issues and PRs related to the benchmark subsystem. labels Jul 8, 2016
@mscdex
Copy link
Contributor

mscdex commented Jul 9, 2016

I'm guessing it wouldn't be sufficient to just close the socket to allow the process to exit naturally or ?

@Trott
Copy link
Member Author

Trott commented Jul 9, 2016

@mscdex To do that you need to handle stopping the firehose to the socket first and letting all those messages finish arriving or else you get an error after closing the socket and the subsequent benchmark runs never happen. Probably do-able, but I'm not sure the extra complexity will be worth it. Maybe? If anyone wants to put together a competing implementation that does it that way, please go for it.

@mscdex
Copy link
Contributor

mscdex commented Jul 9, 2016

This change is fine with me, I was just curious. LGTM

@Trott
Copy link
Member Author

Trott commented Jul 11, 2016

@mscdex
Copy link
Contributor

mscdex commented Jul 11, 2016

Argh, I forgot I already made this change as part of #7311. It would be nice if we could get that to land since it is more comprehensive. Any input on that PR would be great.

@Trott
Copy link
Member Author

Trott commented Jul 12, 2016

Closing in favor of #7311

@Trott Trott closed this Jul 12, 2016
@Trott Trott deleted the fix-exit branch January 13, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Issues and PRs related to the benchmark subsystem. dgram Issues and PRs related to the dgram subsystem / UDP.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants