Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bootstrap: use a context snapshotted with primordials in workers #42867

Merged
merged 1 commit into from
May 5, 2022

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Apr 25, 2022

First commit is taken from #42864

Refs: #35711

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 25, 2022
@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member

#42864 is landed. So a rebase is needed :)

@joyeecheung joyeecheung force-pushed the worker-snapshot-2 branch from 8e964b1 to b6c0061 Compare May 2, 2022 08:02
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95
Copy link
Contributor

aduh95 commented May 3, 2022

@joyeecheung
Copy link
Member Author

Benchmark result:

01:07:45                                                                                   confidence improvement accuracy (*)   (**)  (***)
01:07:45 misc/startup.js mode='process' script='benchmark/fixtures/require-builtins' dur=1                -0.73 %       ±1.29% ±1.72% ±2.24%
01:07:45 misc/startup.js mode='process' script='benchmark/fixtures/require-cachable' dur=1                -0.53 %       ±1.23% ±1.63% ±2.12%
01:07:45 misc/startup.js mode='process' script='test/fixtures/semicolon' dur=1                            -0.42 %       ±1.58% ±2.10% ±2.74%
01:07:45 misc/startup.js mode='worker' script='benchmark/fixtures/require-builtins' dur=1         ***      6.86 %       ±2.78% ±3.70% ±4.83%
01:07:45 misc/startup.js mode='worker' script='benchmark/fixtures/require-cachable' dur=1         ***      6.00 %       ±3.36% ±4.46% ±5.81%
01:07:45 misc/startup.js mode='worker' script='test/fixtures/semicolon' dur=1                     ***      7.42 %       ±3.13% ±4.18% ±5.45%

The last CI run looks okay (failures seemed unrelated). Can I have some review please? @nodejs/startup and @nodejs/workers

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label May 5, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 5, 2022
@nodejs-github-bot nodejs-github-bot merged commit f7d658a into nodejs:master May 5, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in f7d658a

RafaelGSS pushed a commit that referenced this pull request May 10, 2022
PR-URL: #42867
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request May 10, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants