GeezerVison

Copy link

Copy Markdown

Member

addaleax approved these changes Nov 15, 2018

View reviewed changes

refack reviewed Nov 16, 2018

View reviewed changes

refack reviewed Nov 16, 2018

View reviewed changes

refack reviewed Nov 16, 2018

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

fhinkel approved these changes Nov 16, 2018

View reviewed changes

joyeecheung force-pushed the map branch from d26e166 to 9b60fd6 Compare November 16, 2018 14:05

joyeecheung force-pushed the map branch from 9b60fd6 to 5a81e0d Compare November 16, 2018 15:22

TimothyGu reviewed Nov 16, 2018

View reviewed changes

                                                                                   confidence improvement accuracy (*)   (**)  (***)
 misc/startup.js mode='process' script='benchmark/fixtures/require-cachable' dur=1                -0.95 %       ±1.05% ±1.40% ±1.82%
 misc/startup.js mode='process' script='test/fixtures/semicolon' dur=1                      *      1.46 %       ±1.23% ±1.64% ±2.14%
 misc/startup.js mode='worker' script='benchmark/fixtures/require-cachable' dur=1                 -0.35 %       ±0.85% ±1.13% ±1.47%
 misc/startup.js mode='worker' script='test/fixtures/semicolon' dur=1                             -0.09 %       ±1.06% ±1.41% ±1.83%

                                                                                   confidence improvement accuracy (*)   (**)  (***)
 misc/startup.js mode='process' script='benchmark/fixtures/require-cachable' dur=1         **     -1.58 %       ±1.18% ±1.57% ±2.04%
 misc/startup.js mode='process' script='test/fixtures/semicolon' dur=1                             0.13 %       ±1.37% ±1.82% ±2.37%
 misc/startup.js mode='worker' script='benchmark/fixtures/require-cachable' dur=1         ***     -2.43 %       ±0.86% ±1.14% ±1.49%
 misc/startup.js mode='worker' script='test/fixtures/semicolon' dur=1                              0.12 %       ±0.87% ±1.16% ±1.51%

joyeecheung force-pushed the map branch from 470d91d to e1c5eb8 Compare November 17, 2018 02:13

TimothyGu approved these changes Nov 18, 2018

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell approved these changes Nov 19, 2018

View reviewed changes

Instead of putting the source code and the cache in v8::Objects,
put them in per-process std::maps. This has the following benefits:

- It's slightly lighter in weight compared to storing things on the
  v8 heap. Also it may be slightly faster since the preivous v8::Object
  is already in dictionary mode - though the difference is very small
  given the number of native modules is limited.
- The source and code cache generation templates are now much simpler
  since they just initialize static arrays and manipulate STL
  constructs.
- The static native module data can be accessed independently of any
  Environment or Isolate, and it's easy to look them up from the
  C++'s side.
- It's now impossible to mutate the source code used to compile
  native modules from the JS land since it's completely separate
  from the v8 heap. We can still get the constant strings from
  process.binding('natives') but that's all.

A few drive-by fixes:

- Remove DecorateErrorStack in LookupAndCompile - We don't need to
  capture the exception to decorate when we encounter
  errors during native module compilation, as those errors should be
  syntax errors and v8 is able to decorate them well. We use
  CompileFunctionInContext so there is no need to worry about
  wrappers either.
- The code cache could be rejected when node is started with v8 flags.
  Instead of aborting in that case, simply keep a record in the
  native_module_without_cache set.
- Refactor js2c.py a bit, reduce code duplication and inline Render()
  to make the one-byte/two-byte special treatment easier to read.

PR-URL: #24384
Fixes: https://github.com/Remove
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Instead of putting the source code and the cache in v8::Objects,
put them in per-process std::maps. This has the following benefits:

- It's slightly lighter in weight compared to storing things on the
  v8 heap. Also it may be slightly faster since the preivous v8::Object
  is already in dictionary mode - though the difference is very small
  given the number of native modules is limited.
- The source and code cache generation templates are now much simpler
  since they just initialize static arrays and manipulate STL
  constructs.
- The static native module data can be accessed independently of any
  Environment or Isolate, and it's easy to look them up from the
  C++'s side.
- It's now impossible to mutate the source code used to compile
  native modules from the JS land since it's completely separate
  from the v8 heap. We can still get the constant strings from
  process.binding('natives') but that's all.

A few drive-by fixes:

- Remove DecorateErrorStack in LookupAndCompile - We don't need to
  capture the exception to decorate when we encounter
  errors during native module compilation, as those errors should be
  syntax errors and v8 is able to decorate them well. We use
  CompileFunctionInContext so there is no need to worry about
  wrappers either.
- The code cache could be rejected when node is started with v8 flags.
  Instead of aborting in that case, simply keep a record in the
  native_module_without_cache set.
- Refactor js2c.py a bit, reduce code duplication and inline Render()
  to make the one-byte/two-byte special treatment easier to read.

PR-URL: #24384
Fixes: https://github.com/Remove
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Instead of putting the source code and the cache in v8::Objects,
put them in per-process std::maps. This has the following benefits:

- It's slightly lighter in weight compared to storing things on the
  v8 heap. Also it may be slightly faster since the preivous v8::Object
  is already in dictionary mode - though the difference is very small
  given the number of native modules is limited.
- The source and code cache generation templates are now much simpler
  since they just initialize static arrays and manipulate STL
  constructs.
- The static native module data can be accessed independently of any
  Environment or Isolate, and it's easy to look them up from the
  C++'s side.
- It's now impossible to mutate the source code used to compile
  native modules from the JS land since it's completely separate
  from the v8 heap. We can still get the constant strings from
  process.binding('natives') but that's all.

A few drive-by fixes:

- Remove DecorateErrorStack in LookupAndCompile - We don't need to
  capture the exception to decorate when we encounter
  errors during native module compilation, as those errors should be
  syntax errors and v8 is able to decorate them well. We use
  CompileFunctionInContext so there is no need to worry about
  wrappers either.
- The code cache could be rejected when node is started with v8 flags.
  Instead of aborting in that case, simply keep a record in the
  native_module_without_cache set.
- Refactor js2c.py a bit, reduce code duplication and inline Render()
  to make the one-byte/two-byte special treatment easier to read.

PR-URL: #24384
Fixes: https://github.com/Remove
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

nodejs/node#24384

Instead of putting the source code and the cache in v8::Objects,
put them in per-process std::maps. This has the following benefits:

- It's slightly lighter in weight compared to storing things on the
  v8 heap. Also it may be slightly faster since the preivous v8::Object
  is already in dictionary mode - though the difference is very small
  given the number of native modules is limited.
- The source and code cache generation templates are now much simpler
  since they just initialize static arrays and manipulate STL
  constructs.
- The static native module data can be accessed independently of any
  Environment or Isolate, and it's easy to look them up from the
  C++'s side.
- It's now impossible to mutate the source code used to compile
  native modules from the JS land since it's completely separate
  from the v8 heap. We can still get the constant strings from
  process.binding('natives') but that's all.

A few drive-by fixes:

- Remove DecorateErrorStack in LookupAndCompile - We don't need to
  capture the exception to decorate when we encounter
  errors during native module compilation, as those errors should be
  syntax errors and v8 is able to decorate them well. We use
  CompileFunctionInContext so there is no need to worry about
  wrappers either.
- The code cache could be rejected when node is started with v8 flags.
  Instead of aborting in that case, simply keep a record in the
  native_module_without_cache set.
- Refactor js2c.py a bit, reduce code duplication and inline Render()
  to make the one-byte/two-byte special treatment easier to read.

PR-URL: nodejs#24384
Fixes: https://github.com/Remove
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>