optimize(core): move internal js src off heap #9713

AaronO · 2021-03-06T23:46:19Z

This PR explores moving the internal JS code (bundled in deno) off the heap, as suggested by #9675

Impact

Reduce per-isolate heap by ~1MB, reducing overall memory usage (especially with many workers), with no real downside or compromise.

This should also reduce snapshot sizes, resulting in faster snapshot restores and a smaller deno binary (these will likely be minor and weren't the core motivation)

Subtasks

ASCII JS code
- Ensure internal JS files are ASCII
- Add a lint / CI test check to ensure files remain ASCII
Allow executing JS code via external v8::Strings
- Implement external strings in rusty_v8 (Initial support for external v8 strings rusty_v8#641)
- Implement JsRuntime::execute_static
Work with snapshots
- Implement support for string ExternalReferences in v8, so they can be captured and restored in snapshots
- Register internal src references with the snapshot creator (maybe as they're passed through execute_static ?)

Notes

Non-ASCII files found with (ignores test and example files):

find core runtime op_crates -name "*.[jt]s" | grep -v "examples" | grep -v "_test" | xargs file | grep -v ASCII

Non-ASCII characters in each file were then found with this regex: [^\x00-\x7F]

Non-ASCII files and context:

runtime/js/02_console.js: the non-ASCII chars are the tableChars (L38)
op_crates/web/02_event.js: there's a single utf8 apostrophe ’ vs ' in comments (L919)
op_crates/web/21_filereader.js: there are multiple utf8 apostrophes in comments (same as above)
op_crates/fetch/26_fetch.js: same deal, same apostrophes
op_crates/web/lib.deno_web.d.ts: used § in comment

Since v8 only excepts one-byte external strings in ASCII/Latin-1

ry

Awesome! This is a great simple optimization

ry · 2021-03-07T01:08:42Z

op_crates/web/02_event.js

@@ -916,7 +916,7 @@
          // If signal is not null and its aborted flag is set, then return.
          return;
        } else {
-          // If listener’s signal is not null, then add the following abort


This character non-ascii?

Really subtle but this is a right single quotation mark (0x2019).
A quick s/’/'/g and we should be good.

ry · 2021-03-09T01:47:04Z

rusty_v8 upgraded in #9725 with your external string APIs

AaronO · 2021-03-09T05:37:03Z

Thanks @ry. Btw, this will still require some further changes in rusty_v8, specifically for:

Implement support for string ExternalReferences in v8, so they can be captured and restored in snapshots

So external strings work, just not with snapshots yet. So I wouldn't bump rusty_v8 solely for this PR without the ExternalReference changes.

Locally I've made the necessary changes to register the external refs (beyond just bindings::EXTERNAL_REFERENCES) and pass in these string pointers, everything runs but it doesn't impact the heap-usage as intended.

So this will require a little more investigating on my end.

jbergstroem · 2021-03-10T17:01:38Z

Perhaps there should be a CI job to check for this in future code changes. Add to linting?

AaronO · 2021-03-10T17:04:45Z

@jbergstroem It's planned, see the subtask:

Add a lint / CI test check to ensure files remain ASCII

bartlomieju · 2021-07-06T12:43:30Z

I will add relevant bindings to rusty_v8 to support this

stale · 2021-09-04T13:26:37Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

magurotuna · 2021-09-04T18:28:50Z

Completed to implement a new lint rule in denoland/deno_lint#837.
Quickly running the rule on ext, runtime and core, I've got the following errors.

ext

Found 22 problems. I'm worried that non-ASCII characters used in 02_console.js are required to implement console.table.

click to expand

error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:129:20
    |
129 |     middleMiddle: "─",
    |                    ^
    |
    = help: `─` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:130:17
    |
130 |     rowMiddle: "┼",
    |                 ^
    |
    = help: `┼` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:131:16
    |
131 |     topRight: "┐",
    |                ^
    |
    = help: `┐` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:132:15
    |
132 |     topLeft: "┌",
    |               ^
    |
    = help: `┌` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:133:18
    |
133 |     leftMiddle: "├",
    |                  ^
    |
    = help: `├` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:134:17
    |
134 |     topMiddle: "┬",
    |                 ^
    |
    = help: `┬` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:135:19
    |
135 |     bottomRight: "┘",
    |                   ^
    |
    = help: `┘` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:136:18
    |
136 |     bottomLeft: "└",
    |                  ^
    |
    = help: `└` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:137:20
    |
137 |     bottomMiddle: "┴",
    |                    ^
    |
    = help: `┴` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:138:19
    |
138 |     rightMiddle: "┤",
    |                   ^
    |
    = help: `┤` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:139:12
    |
139 |     left: "│ ",
    |            ^
    |
    = help: `│` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:140:14
    |
140 |     right: " │",
    |              ^
    |
    = help: `│` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/console/02_console.js:141:15
    |
141 |     middle: " │ ",
    |               ^
    |
    = help: `│` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/02_event.js:927:25
    |
927 |           // If listener’s signal is not null, then add the following abort
    |                         ^
    |
    = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:64:18
   |
64 |       // 1. If fr’s state is "loading", throw an InvalidStateError DOMException.
   |                  ^
   |
   = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:71:19
   |
71 |       // 2. Set fr’s state to "loading".
   |                   ^
   |
   = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:73:19
   |
73 |       // 3. Set fr’s result to null.
   |                   ^
   |
   = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:75:19
   |
75 |       // 4. Set fr’s error to null.
   |                   ^
   |
   = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:149:29
    |
149 |                 // 1. Set fr’s state to "done".
    |                             ^
    |
    = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:151:87
    |
151 |                 // 2. Let result be the result of package data given bytes, type, blob’s type, and encodingName.
    |                                                                                       ^
    |
    = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:226:28
    |
226 |                 // 5. If fr’s state is not "loading", fire a progress event called loadend at the fr.
    |                            ^
    |
    = help: `’` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
   --> /Users/yusuktan/Repos/github.com/magurotuna/deno/ext/web/10_filereader.js:253:22
    |
253 |               //If fr’s state is not "loading", fire a progress event called loadend at fr.
    |                      ^
    |
    = help: `’` is not an ASCII. Consider replacing it
Found 22 problems

runtime

No non-ASCII characters are found.

core

Found 10 problems, but all of them are in test file encode_decode_test.js

click to expand

error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:39:50
   |
39 |   assertArrayEquals(Array.from(Deno.core.encode("𝓽𝓮𝔁𝓽")), fixture1);
   |                                                  ^
   |
   = help: `𝓽` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:39:51
   |
39 |   assertArrayEquals(Array.from(Deno.core.encode("𝓽𝓮𝔁𝓽")), fixture1);
   |                                                   ^
   |
   = help: `𝓮` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:39:52
   |
39 |   assertArrayEquals(Array.from(Deno.core.encode("𝓽𝓮𝔁𝓽")), fixture1);
   |                                                    ^
   |
   = help: `𝔁` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:39:53
   |
39 |   assertArrayEquals(Array.from(Deno.core.encode("𝓽𝓮𝔁𝓽")), fixture1);
   |                                                     ^
   |
   = help: `𝓽` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:48:58
   |
48 |   assert(Deno.core.decode(new Uint8Array(fixture1)) === "𝓽𝓮𝔁𝓽");
   |                                                          ^
   |
   = help: `𝓽` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:48:59
   |
48 |   assert(Deno.core.decode(new Uint8Array(fixture1)) === "𝓽𝓮𝔁𝓽");
   |                                                           ^
   |
   = help: `𝓮` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:48:60
   |
48 |   assert(Deno.core.decode(new Uint8Array(fixture1)) === "𝓽𝓮𝔁𝓽");
   |                                                            ^
   |
   = help: `𝔁` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:48:61
   |
48 |   assert(Deno.core.decode(new Uint8Array(fixture1)) === "𝓽𝓮𝔁𝓽");
   |                                                             ^
   |
   = help: `𝓽` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:49:64
   |
49 |   assert(Deno.core.decode(new Uint8Array(fixture2)) === "Hello �� World");
   |                                                                ^
   |
   = help: `�` is not an ASCII. Consider replacing it
error[prefer-ascii]: Non-ASCII characters are not allowed
  --> /Users/yusuktan/Repos/github.com/magurotuna/deno/core/encode_decode_test.js:49:65
   |
49 |   assert(Deno.core.decode(new Uint8Array(fixture2)) === "Hello �� World");
   |                                                                 ^
   |
   = help: `�` is not an ASCII. Consider replacing it
Found 10 problems

CC @AaronO

AaronO · 2021-09-04T18:49:47Z

@magurotuna Great job !

A nice additional improvement would be to show the escaped \x... value

CLAassistant · 2021-10-15T05:57:52Z

All committers have signed the CLA.

ry · 2021-11-09T21:10:11Z

Closing because stale. Open a new PR if this work continues.

AaronO added 4 commits March 6, 2021 22:52

Convert non-ASCII js/ts files to ASCII

af78751

Since v8 only excepts one-byte external strings in ASCII/Latin-1

fmt

612a577

core: implement JsRuntime::execute_static

1c08301

fmt

e16719f

ry reviewed Mar 7, 2021

View reviewed changes

AaronO mentioned this pull request Mar 9, 2021

Improving op_crate API and registering ops #9738

Closed

AaronO mentioned this pull request Mar 15, 2021

core: introduce extensions #9800

Merged

24 tasks

bartlomieju added this to the 1.10.0 milestone Apr 16, 2021

AaronO mentioned this pull request May 2, 2021

refactor(snapshots): to their own crate #10472

Closed

6 tasks

bartlomieju removed this from the 1.10.0 milestone Jun 6, 2021

bartlomieju self-assigned this Jul 6, 2021

ry mentioned this pull request Sep 1, 2021

Support string in ExternalReference denoland/rusty_v8#765

Open

magurotuna mentioned this pull request Sep 1, 2021

New lint: prefer-ascii for Deno's internal JS denoland/deno_lint#832

Closed

stale bot added the stale label Sep 4, 2021

stale bot removed the stale label Sep 4, 2021

magurotuna mentioned this pull request Sep 9, 2021

prefer-ascii: Show \x... value on non-ascii character in hint denoland/deno_lint#852

Closed

ry closed this Nov 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optimize(core): move internal js src off heap #9713

optimize(core): move internal js src off heap #9713

AaronO commented Mar 6, 2021 •

edited

Loading

ry left a comment

ry Mar 7, 2021

caspervonb Mar 7, 2021

ry commented Mar 9, 2021 •

edited

Loading

AaronO commented Mar 9, 2021

jbergstroem commented Mar 10, 2021

AaronO commented Mar 10, 2021

bartlomieju commented Jul 6, 2021

stale bot commented Sep 4, 2021

magurotuna commented Sep 4, 2021 •

edited

Loading

AaronO commented Sep 4, 2021

CLAassistant commented Oct 15, 2021 •

edited

Loading

ry commented Nov 9, 2021

optimize(core): move internal js src off heap #9713

optimize(core): move internal js src off heap #9713

Conversation

AaronO commented Mar 6, 2021 • edited Loading

Impact

Subtasks

Notes

ry left a comment

Choose a reason for hiding this comment

ry Mar 7, 2021

Choose a reason for hiding this comment

caspervonb Mar 7, 2021

Choose a reason for hiding this comment

ry commented Mar 9, 2021 • edited Loading

AaronO commented Mar 9, 2021

jbergstroem commented Mar 10, 2021

AaronO commented Mar 10, 2021

bartlomieju commented Jul 6, 2021

stale bot commented Sep 4, 2021

magurotuna commented Sep 4, 2021 • edited Loading

ext

runtime

core

AaronO commented Sep 4, 2021

CLAassistant commented Oct 15, 2021 • edited Loading

ry commented Nov 9, 2021

AaronO commented Mar 6, 2021 •

edited

Loading

ry commented Mar 9, 2021 •

edited

Loading

magurotuna commented Sep 4, 2021 •

edited

Loading

CLAassistant commented Oct 15, 2021 •

edited

Loading