Home Original page

src: use object to pass `Environment` to functions by addaleax · Pull Request #26382 · nodejs/node

@addaleax 

Use a `v8::Object` with an internal field, rather than a
`v8::External`.

On a `GetReturnValue().Set(Environment::GetCurrent(args) == nullptr)`
noop function, this benchmarks as a ~60 % speedup, as calls to
`obj->GetAlignedPointerFromInternalField()` can be inlined.

This also makes breaking up some pieces of the `Environment` class
into per-native-binding data easier, if we want to pursue that path
in the future.

@nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

lib / src

Issues and PRs related to general changes in the lib or src directory.

labels

Mar 1, 2019

bnoordhuis

@addaleax addaleax added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Mar 2, 2019

addaleax added a commit that referenced this pull request

Mar 5, 2019 
Use a `v8::Object` with an internal field, rather than a
`v8::External`.

On a `GetReturnValue().Set(Environment::GetCurrent(args) == nullptr)`
noop function, this benchmarks as a ~60 % speedup, as calls to
`obj->GetAlignedPointerFromInternalField()` can be inlined and
the field is stored with one level of indirection less.

This also makes breaking up some pieces of the `Environment` class
into per-native-binding data easier, if we want to pursue that path
in the future.

PR-URL: #26382
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request

Mar 12, 2019 
Use a `v8::Object` with an internal field, rather than a
`v8::External`.

On a `GetReturnValue().Set(Environment::GetCurrent(args) == nullptr)`
noop function, this benchmarks as a ~60 % speedup, as calls to
`obj->GetAlignedPointerFromInternalField()` can be inlined and
the field is stored with one level of indirection less.

This also makes breaking up some pieces of the `Environment` class
into per-native-binding data easier, if we want to pursue that path
in the future.

PR-URL: nodejs#26382
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>