Skip to content

lib: make navigator properties lazy#53649

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
jasnell:make-navigator-properties-lazy
Jul 4, 2024
Merged

lib: make navigator properties lazy#53649
nodejs-github-bot merged 1 commit intonodejs:mainfrom
jasnell:make-navigator-properties-lazy

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented Jun 30, 2024

Noticed in some benchmarking/profiling that the Navigator object constructor was rather expensive and slow due to initialization of properties during construction. It makes more sense for these to be lazily initialized on first access.

Flamegraphs... of `node -pe "navigator"

Before:
image

After:
image

@jasnell jasnell requested review from anonrig and mcollina June 30, 2024 07:48
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 30, 2024

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 30, 2024
@jasnell jasnell added the performance Issues and PRs related to the performance of Node.js. label Jun 30, 2024
@jasnell jasnell force-pushed the make-navigator-properties-lazy branch from 5a3b5a3 to 3d2e10f Compare June 30, 2024 07:53
@jasnell
lib: make navigator properties lazy
875952b 
Noticed in some benchmarking/profiling that the Navigator object
constructor was rather expensive and slow due to initialization
of properties during construction. It makes more sense for these
to be lazily initialized on first access.
@jasnell jasnell force-pushed the make-navigator-properties-lazy branch from 3d2e10f to 875952b Compare June 30, 2024 07:56
jasnell
jasnell commented Jun 30, 2024
View reviewed changes
mcollina
mcollina approved these changes Jun 30, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 30, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 4, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/60054/

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 4, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 4, 2024
@nodejs-github-bot nodejs-github-bot merged commit 8040994 into nodejs:main Jul 4, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 4, 2024

Landed in 8040994

ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version, arch, or plat…
64481f5 
…form

Preserves most of nodejs#53649, except for `#language` due to the lack of Intl primordials.
@ljharb ljharb mentioned this pull request Jul 8, 2024
Merged
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
917e744 
Preserves most of nodejs#53649, except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
1aa58ab 
Preserves most of nodejs#53649, except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
4002f74 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
c45c12c 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
5794fb6 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
f35af3d 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
77899c0 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
0f7da61 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
0acb361 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 8, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
263e61d 
Preserves most of nodejs#53649,
except for `#language` due to the lack of Intl primordials.
ljharb added a commit to ljharb/node that referenced this pull request Jul 9, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
d5ccfd4 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 9, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
2b020c8 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 9, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
35eb2f7 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 11, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
fa90968 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 11, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
f3c4ba5 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 11, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
abf95d3 
Preserves nodejs#53649.
aduh95 pushed a commit that referenced this pull request Jul 12, 2024
@jasnell @aduh95
lib: make navigator properties lazy
1f6fcd6 
Noticed in some benchmarking/profiling that the Navigator object
constructor was rather expensive and slow due to initialization
of properties during construction. It makes more sense for these
to be lazily initialized on first access.

PR-URL: #53649
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@aduh95 aduh95 mentioned this pull request Jul 12, 2024
Merged
ljharb added a commit to ljharb/node that referenced this pull request Jul 15, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
8a25835 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 15, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
42edb23 
Preserves nodejs#53649.
ljharb added a commit to ljharb/node that referenced this pull request Jul 15, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
e0437ed 
Preserves nodejs#53649.
nodejs-github-bot pushed a commit that referenced this pull request Jul 15, 2024
@ljharb
lib: make navigator not runtime-lookup process.version/arch/platform
04e08ad 
Preserves #53649.

PR-URL: #53765
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
@jasnell @aduh95
lib: make navigator properties lazy
0f146aa 
Noticed in some benchmarking/profiling that the Navigator object
constructor was rather expensive and slow due to initialization
of properties during construction. It makes more sense for these
to be lazily initialized on first access.

PR-URL: #53649
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
@ljharb @aduh95
lib: make navigator not runtime-lookup process.version/arch/platform
d1a6749 
Preserves #53649.

PR-URL: #53765
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 16, 2024
@ljharb @aduh95
lib: make navigator not runtime-lookup process.version/arch/platform
f57d3ce 
Preserves #53649.

PR-URL: #53765
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
@ljharb @ehsankhfr
lib: make navigator not runtime-lookup process.version/arch/platform
47ae18c 
Preserves nodejs#53649.

PR-URL: nodejs#53765
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@panva panva panva approved these changes

@benjamingr benjamingr benjamingr approved these changes

@anonrig anonrig anonrig approved these changes

@tniessen tniessen tniessen approved these changes

@MoLow MoLow MoLow 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. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@jasnell @nodejs-github-bot @mcollina @panva @benjamingr @anonrig @tniessen @MoLow @targos @richardlau