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.

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

perf: don't add unload event listener #18082

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

bartlomieju
Copy link
Member

This commit changes how "unload" event is handled - before
this commit an event listener was added unconditionally in
the runtime bootstrapping function, which for some reason was
very expensive (0.3ms). Instead of adding an event listener,
a check was added to "dispatchEvent" function that performs
the same action (so it's only called if there's an event dispatched).

@bartlomieju bartlomieju marked this pull request as ready for review March 9, 2023 00:35
@bartlomieju bartlomieju requested a review from littledivy March 9, 2023 00:35
@bartlomieju bartlomieju enabled auto-merge (squash) March 9, 2023 00:35
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

@bartlomieju bartlomieju merged commit 0f9df73 into denoland:main Mar 9, 2023
@bartlomieju bartlomieju deleted the dispatch_unload_event branch March 9, 2023 01:13
kt3k pushed a commit that referenced this pull request Mar 10, 2023
This commit changes how "unload" event is handled - before
this commit an event listener was added unconditionally in
the runtime bootstrapping function, which for some reason was
very expensive (0.3ms). Instead of adding an event listener,
a check was added to "dispatchEvent" function that performs
the same action (so it's only called if there's an event dispatched).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants