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

build: re-enable custom V8 snapshot by default #28181

Closed
wants to merge 2 commits into from
Closed

build: re-enable custom V8 snapshot by default #28181

wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Jun 11, 2019 •
edited
Loading

tools: assert that the snapshot can be rehashed in node_mksnapshot

build: turn on custom V8 snapshot by default

This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from --with-node-snapshot
to --without-node-snapshot to be consistent with--without-snapshot
which is used for the default V8 snapshot.

Refs: #27365
Refs: #17058

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@joyeecheung
tools: assert that the snapshot can be rehashed in node_mksnapshot
7d917d3
@joyeecheung
build: turn on custom V8 snapshot by default
c1a2c64 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 11, 2019 •
edited
Loading

Sadly, an error occurred when I tried to trigger a build. :(
CI: https://ci.nodejs.org/job/node-test-pull-request/23800/
CI: https://ci.nodejs.org/job/node-test-pull-request/23815/

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Jun 11, 2019
@joyeecheung
Copy link
Member Author

pummel/test-hash-seed CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/7398/

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 11, 2019
bnoordhuis
bnoordhuis approved these changes Jun 12, 2019
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refs: #27365
Refs: #28181

That second Refs: is rather self-referential. ^^

@joyeecheung joyeecheung mentioned this pull request Jun 12, 2019
Closed
@joyeecheung
Copy link
Member Author

@bnoordhuis Oops, it's supposed to be #17058 thanks for catching that.

@BridgeAR BridgeAR mentioned this pull request Jun 12, 2019
Closed
@joyeecheung
Copy link
Member Author

Landed in 28d3f19

joyeecheung added a commit that referenced this pull request Jun 17, 2019
@joyeecheung
tools: assert that the snapshot can be rehashed in node_mksnapshot
b6e41f0 
PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
joyeecheung added a commit that referenced this pull request Jun 17, 2019
@joyeecheung
build: turn on custom V8 snapshot by default
28d3f19 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.

PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
@joyeecheung @BridgeAR
tools: assert that the snapshot can be rehashed in node_mksnapshot
9085412 
PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
@joyeecheung @BridgeAR
build: turn on custom V8 snapshot by default
2201484 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.

PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
Merged
targos pushed a commit that referenced this pull request Jun 18, 2019
@joyeecheung @targos
tools: assert that the snapshot can be rehashed in node_mksnapshot
aeda0c3 
PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 18, 2019
@joyeecheung @targos
build: turn on custom V8 snapshot by default
32b0803 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.

PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR added the notable-change PRs with changes that should be highlighted in changelogs. label Jun 20, 2019
BridgeAR added a commit that referenced this pull request Jun 26, 2019
@BridgeAR
2019-06-18, Version 12.5.0 (Current)
4360273 
Notable changes:

This release contains `semver-major` commits. These are in fact not
`semver-major` due to follow-up commits that remove all breaking changes.

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit that referenced this pull request Jun 26, 2019
@BridgeAR
2019-06-18, Version 12.5.0 (Current)
65829a2 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit that referenced this pull request Jun 26, 2019
@BridgeAR
2019-06-26, Version 12.5.0 (Current)
f5e0b17 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit that referenced this pull request Jun 27, 2019
@BridgeAR
2019-06-27, Version 12.5.0 (Current)
01b404f 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    #28181
* deps:
  * Updated `V8` to 7.5.288.22 #27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c #28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure #27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    #27851
* report:
  * The cpu info got added to the report output
    #28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    #24260
* tools,gyp:
  * Introduce MSVS 2019 #27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      #28059
      #28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines #28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated #28021

PR-URL: #28268
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jun 27, 2019
@BridgeAR
2019-06-27, Version 12.5.0 (Current)
af3cc71 
Notable changes:

* build:
  * The startup time is reduced by enabling V8 snapshots by default
    nodejs#28181
* deps:
  * Updated `V8` to 7.5.288.22 nodejs#27375
    * The numeric separator (v8.dev/features/numeric-separators) feature is now
      enabled by default
  * Updated `OpenSSL` to 1.1.1c nodejs#28211
* inspector:
  * The `--inspect-publish-uid` flag was added to specify ways of the inspector
    web socket url exposure nodejs#27741
* n-api:
  * Accessors on napi_define_* are now ECMAScript-compliant
    nodejs#27851
* report:
  * The cpu info got added to the report output
    nodejs#28188
* src:
  * Restore the original state of the stdio file descriptors on exit to prevent
    leaving stdio in raw or non-blocking mode
    nodejs#24260
* tools,gyp:
  * Introduce MSVS 2019 nodejs#27375
* util:
  * inspect:
    * Array grouping became more compact and uses more columns than before
      nodejs#28059
      nodejs#28070
    * Long strings will not be split at 80 characters anymore. Instead they will
      be split on new lines nodejs#28055
* worker:
  * `worker.terminate()` now returns a promise and using the callback is
    deprecated nodejs#28021

PR-URL: nodejs#28268
@styfle styfle mentioned this pull request Jun 27, 2019
Closed
@jasonkuhrt jasonkuhrt mentioned this pull request Mar 20, 2020
Open
@kissofire kissofire mentioned this pull request Jul 22, 2022
Open
@wedataintelligence wedataintelligence mentioned this pull request Jul 22, 2022
Open
@ron-a20 ron-a20 mentioned this pull request Jul 22, 2022
Open
@Dmarch28 Dmarch28 mentioned this pull request Jul 22, 2022
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this pull request Jul 22, 2022
Open
@137717unity 137717unity mentioned this pull request Jul 22, 2022
Open
@baby636 baby636 mentioned this pull request Jul 22, 2022
Open
@molllyn1 molllyn1 mentioned this pull request Jul 23, 2022
Open
@joyeecheung joyeecheung mentioned this pull request Apr 20, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@addaleax addaleax addaleax approved these changes

@targos targos targos approved these changes

@devsnek devsnek devsnek approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. notable-change PRs with changes that should be highlighted in changelogs. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@joyeecheung @nodejs-github-bot @refack @bnoordhuis @jasnell @Trott @addaleax @targos @devsnek @BridgeAR