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(core) Reduce script name and content copies #18298

Merged
merged 8 commits into from
Mar 21, 2023
Merged

Conversation

mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Mar 21, 2023

Reduce the number of copies and allocations of script code by carrying around ownership/reference information from creation time.

As an advantage, this allows us to maintain the identity of &'static str-based scripts and use v8's external 1-byte strings (to avoid incorrectly passing non-ASCII strings, debug assert!s gate all string reference paths).

Benchmark results:

Perf improvements -- ~0.1 - 0.2ms faster, but should reduce garbage w/external strings and reduces data copies overall. May also unlock some more interesting optimizations in the future.

This requires adding some generics to functions, but manual monomorphization has been applied (outer/inner function) to avoid code bloat.

@mmastrac mmastrac changed the title [WIP] Reduce script copies [WIP] Reduce script name and context copies Mar 21, 2023
@mmastrac mmastrac changed the title [WIP] Reduce script name and context copies [WIP] perf(core) Reduce script name and context copies Mar 21, 2023
@mmastrac mmastrac changed the title [WIP] perf(core) Reduce script name and context copies perf(core) Reduce script name and context copies Mar 21, 2023
@mmastrac mmastrac marked this pull request as ready for review March 21, 2023 14:50
core/lib.rs Show resolved Hide resolved
core/runtime.rs Outdated Show resolved Hide resolved
core/runtime.rs Outdated Show resolved Hide resolved
core/runtime.rs Outdated Show resolved Hide resolved
@mmastrac mmastrac changed the title perf(core) Reduce script name and context copies perf(core) Reduce script name and content copies Mar 21, 2023
core/runtime.rs Outdated Show resolved Hide resolved
Copy link
Member

@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.

LGTM!

@mmastrac mmastrac enabled auto-merge (squash) March 21, 2023 20:50
@mmastrac mmastrac merged commit 0b4770f into denoland:main Mar 21, 2023
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