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

209 cache has instance #214

Conversation

gabrielschulhof
Copy link
Collaborator

This PR depends on #208, so only the last commit is relevant. The approach is to try and retrieve Symbol.hasInstance the first time around. If that succeeds, store it in env in a Persistent and skip the retrieval the next time around. Otherwise set a boolean flag in env indicating that next time around just use the old-fashioned walk-up-the-prototype-chain implementation.

fhinkel and others added 30 commits March 19, 2017 21:41
Original commit message:
  [debugger] fix switch block source positions.

  The switch statement itself is part of the switch block.
  However, the source position of the statement is outside of
  the block. This leads to confusion for the debugger, if the
  switch block pushes a block context: the current context is
  a block context, but the scope analysis based on the current
  source position tells the debugger that we should be outside
  the scope, so we should have the function context.

  [email protected]
  BUG=v8:6085
  Review-Url: https://codereview.chromium.org/2744213003
  Cr-Commit-Position: refs/heads/master@{#43744}
  Committed: https://chromium.googlesource.com/v8/v8/+/09de9969ccb9bc3bbd667788afad665b309d02f5

Fixes: nodejs/node#11746
PR-URL: nodejs/node#11905
Fixes: nodejs/node#11746
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
PR-URL: nodejs/node#11932
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Prototypes are not strict equal when they are from
different contexts. Therefore, assert.strictEqual()
fails for objects that are created in different
contexts, e.g., in vm.runInContext().

Instead of expecting the prototypes to be equal,
only check the properties of the objects
for equality.

PR-URL: nodejs/node#11862
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
OnRead() calls into JS land which can result in the SSL context object
being destroyed on return.  Check that `ssl_ != nullptr` afterwards.

Fixes: nodejs/node#11885
PR-URL: nodejs/node#11898
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Check the options passed to Socket.prototype.connect() to
validate the type of the lookup property.

PR-URL: nodejs/node#11873
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
This test checks for the different input types for the generation
of UNIX timestamps in the fs module.

PR-URL: nodejs/node#11886
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#11872
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Since file URLs can not have `username/password/port`,
the specification was updated to restrict setting protocol to "file".

Refs: whatwg/url#269
Fixes: nodejs/node#11785
PR-URL: nodejs/node#11887
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Synchronize url-setter-test to upstream.

Refs: web-platform-tests/wpt#5112
PR-URL: nodejs/node#11887
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs/node#11914
Fixes: nodejs/node#11913
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Check that stdin, stdout and stderr are valid file descriptors on
Windows. If not, reopen them with 'nul' file.

Refs: nodejs/node#875
Fixes: nodejs/node#11656
PR-URL: nodejs/node#11863
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
GitHub now renders Markdown in CommonMark where spaces in a link
destination are invalid and need to be escaped.

PR-URL: nodejs/node#11944
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: nodejs/node#11594
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs/node#11594
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Where inclusion of a lengthy URL causes a line to exceed 80 characters
in our code base, do not report the line length as a linting error.

PR-URL: nodejs/node#11890
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
lib/buffer.js uses a function declaration for `Buffer`. So it never
uses an instance of `Buffer` in the global scope. Therefore the
disabling of the `require-buffer` custom rule is not needed. Remove the
comment.

PR-URL: nodejs/node#11906
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Currently test-tls-socket-close will fail if node
was built using --without-ssl. This commit adds a check to
verify is crypto support exists and if not skip this test.

PR-URL: nodejs/node#11911
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This removes common.v8ForceOptimization calls from url and vm benchmark
files.

PR-URL: nodejs/node#11908
Fixes: nodejs/node#11895
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Make it call-site-cwd-independent.

PR-URL: nodejs/node#11904
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Notable Changes:

* buffer:
  - The performance of `.toJSON()` is now up to 2859% faster on average
    (Brian White) nodejs/node#10895

* IPC:
  - Batched writes have been enabled for process IPC on platforms that
    support Unix Domain Sockets. (Alexey Orlenko)
    nodejs/node#10677
  - Performance gains may be up to 40% for some workloads.

* http:
  - Control characters are now always rejected when using
    `http.request()`. (Ben Noordhuis)
    nodejs/node#8923

* node:
  - Heap statistics now support values larger than 4GB. (Ben Noordhuis)
    nodejs/node#10186

PR-URL: nodejs/node#11760
Notable changes

* performance: The performance of several APIs has been improved.
  - `Buffer.compare()` is up to 35% faster on average. (Brian White)
    nodejs/node#10927
  - `buffer.toJSON()` is up to 2859% faster on average. (Brian White)
    nodejs/node#10895
  - `fs.*statSync()` functions are now up to 9.3% faster on average.
    (Brian White) nodejs/node#11522
  - `os.loadavg` is up to 151% faster. (Brian White)
    nodejs/node#11516
  - `process.memoryUsage()` is up to 34% faster. (Brian White)
    nodejs/node#11497
  - `querystring.unescape()` for `Buffer`s is 15% faster on average.
    (Brian White) nodejs/node#10837
  - `querystring.stringify()` is up to 7.8% faster on average.
    (Brian White) nodejs/node#10852
  - `querystring.parse()` is up to 21% faster on average. (Brian White)
    nodejs/node#10874

* IPC:
  - Batched writes have been enabled for process IPC on platforms that
    support Unix Domain Sockets. (Alexey Orlenko)
    nodejs/node#10677
  - Performance gains may be up to 40% for some workloads.

* child_process:
  - `spawnSync` now returns a null `status` when child is terminated by
    a signal. (cjihrig) nodejs/node#11288
  - This fixes the behavior to act like `spawn()` does.

* http:
  - Control characters are now always rejected when using
    `http.request()`. (Ben Noordhuis)
    nodejs/node#8923
  - Debug messages have been added for cases when headers contain
    invalid values. (Evan Lucas)
    nodejs/node#9195

* node:
  - Heap statistics now support values larger than 4GB. (Ben Noordhuis)
    nodejs/node#10186

* timers:
  - Timer callbacks now always maintain order when interacting with
    domain error handling. (John Barboza)
    nodejs/node#10522

PR-URL: nodejs/node#11759
Notable changes:

* deps: Add node-inspect 1.10.6 (Jan Krems) nodejs/node#11869
* inspector: proper WS URLs when bound to 0.0.0.0 (Eugene Ostroukhov) nodejs/node#11850
* tls: fix segfault on destroy after partial read. (Ben Noordhuis) nodejs/node#11898

PR-URL: nodejs/node#11941
The bundled c-ares isn't very suitable for consumption by addons,
isn't kept stable, and isn't exported on windows.

PR-URL: nodejs/node#10283
Refs: nodejs/node-gyp#1055
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs/node#11858
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
The entire `URLSearchParams` class is now fully spec-compliant.

PR-URL: nodejs/node#11858
Fixes: nodejs/node#10821
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
It only returns 0, nor is it likely to have any error conditions in the
future.

PR-URL: nodejs/node#11922
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Replace timer/timeout race with event-based ordering, eliminating test
flakiness.

PR-URL: nodejs/node#11921
Fixes: nodejs/node#11912
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
While `child_process.execFile()` gets called in places in the test
suite, there are no explicit test for it and there are parts of the
implementation that are not covered by tests. This adds a minimal test
that increases (but does not complete) coverage for the implementation.

PR-URL: nodejs/node#11929
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Also add test cases for partial writes and invalid indices.

PR-URL: nodejs/node#11927
Fixes: nodejs/node#8724
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Add a space for minimal readability.

PR-URL: nodejs/node#11925
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
JR McEntee and others added 23 commits April 4, 2017 10:14
This commit updates 3 additional references to Mac OS X in
releases.md to macOS.

PR-URL: nodejs/node#12106
Fixes: nodejs/node#12086
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
At the moment Argv only supports three arguments which fulfilled my
requirements when working on #9163.

This commit adds support for a variable number of arguments. There is
also a no-args constructor that is the equivalent to running "node -p
process.version" which is hopefully alright as a default.

PR-URL: nodejs/node#12166
Ref: nodejs/node#9163
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#12162
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ESLint 3.19.0 allows the specification of selectors that represent
disallowed syntax. Replace our custom rule for timer arguments with a
pair of `no-restricted-syntax` option objects.

PR-URL: nodejs/node#12162
Reviewed-By: Teddy Katz <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Use the remaining listener directly if the array of listeners has only
one element after running `EventEmitter.prototype.removeListener()`.

Advantages:

- Better memory usage and better performance if no new listeners are
  added for the same event.

Disadvantages:

- A new array must be created if new listeners are added for the same
  event.

PR-URL: nodejs/node#12043
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Ron Korving <[email protected]>
About fs.read's 2nd argument, string is invalid.

PR-URL: nodejs/node#12034
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
About setImmediate, the execution timing is after timers currently.

PR-URL: nodejs/node#12034
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Something unidentified at the moment is causing the arrays benchmarks to
deopt when run with the TurboFan compiler. Refactor the test to use an
inner function that can be correctly optimized by TurboFan and
Crankshaft.

PR-URL: nodejs/node#11894
Ref: nodejs/node#11851 (comment)
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
The auth property of a URL is decoded via decodeURIComponent,
which can throw a URIError. The test URL here will trigger this.

Adds documentation on the possible errors url.parse can throw.

PR-URL: nodejs/node#12135
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
look for the actual produced `exe` not just the directory

PR-URL: nodejs/node#12120
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
PR-URL: nodejs/node#11726
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: nodejs/node#11635
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
This test ensures that UTF-8 characters can be used in core JavaScript
modules built into Node's binary.

PR-URL: nodejs/node#11423
Ref: nodejs/node#11129
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This test refactored the original test for mkdtempSync prefix validation
and added the test also for the async function mkdtemp.

PR-URL: nodejs/node#12080
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
PR-URL: nodejs/node#12221
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs/node#11690
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
* Replace `var` by `const`.
* Comment out ellipses.
* Update code example (provide relevant file path, add missing option).

PR-URL: nodejs/node#12171
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
I noticed that few of the print statements in configure have a leading
space with is not consistent with the rest of the file. Not sure if
this intentional or not so creating this commit just to bring it up
just in case.

PR-URL: nodejs/node#12176

Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: mhdawson - Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Currently it is possible to configure using --without-ssl and
--shared-openssl/--openssl-no-asm/openssl-fips without an error
occuring.

The commit add check for these combinations:

$ ./configure --without-ssl --shared-openssl
Error: --without-ssl is incompatible with --shared-openssl

$ ./configure --without-ssl --openssl-no-asm
Error: --without-ssl is incompatible with --openssl-no-asm

$ ./configure --without-ssl --openssl-fips=dummy
Error: --without-ssl is incompatible with --openssl-fips

PR-URL: nodejs/node#12175
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
The v8 n-api implementation had been depending on a one-to-one
relationship between v8 and n-api v8 property attributes.
Remove this dependency and fix coverity scan issue
165845.

PR-URL: nodejs/node#12191
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Coverity was complaining that finalizer was being
leaked in this method, however it should be
freed when the buffer is finalized so I believe
the message is invalid.

Add the required comments to suppress the warning.

PR-URL: nodejs/node#12192
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Hitesh Kanwathirtha <[email protected]>
1. We define struct napi_env__ to include the isolate, the last
exception, and the info about the last error.

2. We instantiate one struct napi_env__ during module registration and
we pass it into the FunctionCallbackInfo for all subsequent entries into
N-API when we create functions/accessors/finalizers.

Once module unloading will be supported we shall have to delete the
napi_env we create during module init.

There is a clear separation between public and private API wrt. env:

1. Public APIs assert that env is not nullptr as their first action.

2. Private APIs need not validate env. They assume it's not nullptr.

PR-URL: nodejs/node#12195
Fixes: nodejs#198
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
This improves the performance of napi_instanceof() by retrieving
Symbol.hasInstance from the global object once and then storing a
persistent reference to it in the env.

Re nodejs#209
Re nodejs#214
@mhdawson
Copy link
Member

landed in main repo closing.

@mhdawson mhdawson closed this Apr 11, 2017
@gabrielschulhof gabrielschulhof deleted the 209-cache-has-instance branch April 20, 2017 12:25
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.