-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
core: don't include_str extension js code #10786
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This speeds up incremental rebuild when only touching JS files by 13-15% Rebuild time after `touch 01_broadcast_channel.js`: main: run 1 49.18s, run 2 50.34s this: run 1 43.12s, run 2 43.19s
lucacasonato
added a commit
to lucacasonato/deno
that referenced
this pull request
May 29, 2021
This speeds up incremental rebuild when only touching JS files by 30% compared to denoland#10786. Rebuild time after touch 01_broadcast_channel.js: main: run 1 49.18s, run 2 50.34s denoland#10786: run 1 43.12s, run 2 43.19s this + denoland#10786: run 1 30.30s, run 2 30.95s
Binary size reduction is around 0.9%. latest canary: 81315944 bytes |
ry
approved these changes
May 29, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work Luca! LGTM
AaronO
added a commit
to AaronO/deno
that referenced
this pull request
May 15, 2022
This reverts commit 10e50a1
AaronO
added a commit
that referenced
this pull request
May 15, 2022
This reverts commit 10e50a1 Alternative to #13217, IMO the tradeoffs made by #10786 aren't worth it. It breaks abstractions (crates being self-contained, deno_core without snapshotting etc...) and causes pain points / gotchas for both embedders & devs for a relatively minimal gain in incremental build time ... Closes #11030
sigmaSd
pushed a commit
to sigmaSd/deno
that referenced
this pull request
May 29, 2022
…enoland#14614) This reverts commit 10e50a1 Alternative to denoland#13217, IMO the tradeoffs made by denoland#10786 aren't worth it. It breaks abstractions (crates being self-contained, deno_core without snapshotting etc...) and causes pain points / gotchas for both embedders & devs for a relatively minimal gain in incremental build time ... Closes denoland#11030
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This speeds up incremental rebuild when only touching JS files by 13-15%
Rebuild time after
touch 01_broadcast_channel.js
:main: run 1 49.18s, run 2 50.34s
this: run 1 43.12s, run 2 43.19s
The reason for this speedup is that
deno_runtime(build.rs)
does notneed to be rebuilt on every js file change now.
Closes #10786