Skip to content

feat: use a single 'performance' global #29323

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
Merged
merged 2 commits into from
May 16, 2025
Merged

feat: use a single 'performance' global #29323

merged 2 commits into from
May 16, 2025

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented May 16, 2025 •
edited
Loading

This commit changes how "performance" global is handled -
instead of having separate "performance" globals for user and npm
code, we now have a single implementation.

This implementation is a de facto Node.js implementation - ie. it contains
additional properties compared to the "Web" implementation.

The two global can be compared using these docs sites:

The benefit of this change is that when you use performance.now() in a
tight loop, the actual runtime of the code significantly improves - this is
because the performance global doesn't have to be "looked up" and
decided which is to be used, the "Web" or the "Node.js" version.

bartlomieju
bartlomieju commented May 16, 2025
View reviewed changes
ext/node/polyfills/perf_hooks.js
Copy link
Member Author

Choose a reason for hiding this comment

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

Temporary solution - I'm not yet sure if we should have two separate files that implement and contribute to "performance" global - ATM I think "yes", because this file uses additional classes that are not available in the "Web" version, but it is to be seen.

bartlomieju
bartlomieju commented May 16, 2025
View reviewed changes
Copy link
Member Author

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

I expect several WPT tests to fail

marvinhagemeister
marvinhagemeister approved these changes May 16, 2025
View reviewed changes
Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

LGTM

littledivy
littledivy approved these changes May 16, 2025
View reviewed changes
Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM

@lucacasonato lucacasonato changed the title feat: Use a single 'performance' global feat: use a single 'performance' global May 16, 2025
@bartlomieju bartlomieju merged commit b41fe8c into denoland:main May 16, 2025
18 checks passed
@bartlomieju bartlomieju deleted the single_performance_global branch May 16, 2025 22:11
@bartlomieju bartlomieju mentioned this pull request May 22, 2025
Merged
@dsherret dsherret mentioned this pull request May 30, 2025
Closed
@bartlomieju bartlomieju mentioned this pull request Jun 2, 2025
Merged
bartlomieju added a commit that referenced this pull request Jun 2, 2025
@bartlomieju
fix(ext/node): add back perf_hooks.markResourceTiming (#29562)
0240433 
Adding back a sham for "perf_hooks.markResourceTiming" that was removed
by accident in #29323; a test was
added too, to ensure it doesn't regress in the future.

Closes #29539
@bartlomieju bartlomieju mentioned this pull request Jun 16, 2025
Open
12 tasks
bartlomieju added a commit that referenced this pull request Jun 24, 2025
@bartlomieju
feat: make 'Buffer' and 'global' available as globals (#29416)
ed0c77e 
Follow up to #29323.

This effort is to reduce a number of different globals available to user
code and npm dependencies. Also improves performance when these globals
are used.

Towards #29703
@bartlomieju bartlomieju mentioned this pull request Jun 24, 2025
Merged
bartlomieju added a commit that referenced this pull request Jun 25, 2025
@bartlomieju
feat: make setImmediate and clearImmediate as globals (#29877)
d4b0245 
Follow up to #29323 and
#29416.

This effort is to reduce a number of different globals available to user
code and npm dependencies. Also improves performance when these globals
are used.
This brings the number of "managed globals" down to 6. 

Towards #29703
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

@lucacasonato lucacasonato lucacasonato approved these changes

@littledivy littledivy littledivy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bartlomieju @marvinhagemeister @lucacasonato @littledivy