Keepalive sockets that are returned to the agent's freesocket pool were
previously capturing a reference to the ClientRequest that initiated the
request.
This commit eliminates that by moving the installation of the socket
listeners to a different function.
PR-URL: https://github.com/nodejs/node/pull/10134
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit makes sure EventEmitter.emit() doesn't get deoptimized by
V8. The deopt happens when accessing out of bound indexes of the
`arguments` object.
This issue has been raised here: #10323 and this specific case might
become a more serious performance issue in upcoming V8 releases.
PR-URL: https://github.com/nodejs/node/pull/10568
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
These changes result in ~50% improvement in the included benchmark.
PR-URL: https://github.com/nodejs/node/pull/10580
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
array.shift() seems to be faster than arrayClone() when the item
to remove is at the front (at least with V8 5.4).
PR-URL: https://github.com/nodejs/node/pull/10572
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This updates util.inspect() to avoid accessing out-of-range indices of
the `arguments` object, which is known to cause optimization bailout.
Based on an average of 10 runs of the benchmark in
`benchmark/util/inspect.js`, this change improves the performance of
`util.inspect` by about 10%.
Relates to https://github.com/nodejs/node/issues/10323
PR-URL: https://github.com/nodejs/node/pull/10569
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove the numbers from the comments to make it clear that
assert does not follow the
[CJS spec](http://wiki.commonjs.org/wiki/Unit_Testing/1.0).
Additionally, clean up the existing comments for consistent
formatting/language and ease of reading.
PR-URL: https://github.com/nodejs/node/pull/10579
Fixes: https://github.com/nodejs/node/issues/9063
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Optimize arrayClone by copying forward.
It's slightly faster (and more readable) to copy array elements
in forward direction. This way it also avoids the ToBoolean and
the postfix count operation.
PR-URL: https://github.com/nodejs/node/pull/10571
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
This patch contains the following changes:
url: make IPv4 parser more spec compliant
* Return int64_t from ParseNumber to prevent overflow for valid big numbers
* Don't throw when there are more than 4 parts (it cannot be an IP
address)
* Correctly interpret the address and don't always throw when there are
numbers > 255
Ref: https://url.spec.whatwg.org/#concept-ipv4-parser
Fixes: https://github.com/nodejs/node/issues/10306
url: percent encode fragment to follow spec change
Ref: https://github.com/whatwg/url/issues/150
Ref: 373dbedbbf
url: fix URL#search setter
The check for empty string must be done before removing the leading '?'.
Ref: https://url.spec.whatwg.org/#dom-url-search
url: set port to null if an empty string is given
This is to follow a spec change.
Ref: https://github.com/whatwg/url/pull/113
url: fix parsing of paths with Windows drive letter
test: update WHATWG URL test fixtures
PR-URL: https://github.com/nodejs/node/pull/10317
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Move non-standard methods to `url` module instead of exposing as
static methods on the `URL` object.
PR-URL: https://github.com/nodejs/node/pull/10512
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This commit uses instanceof instead of Array.isArray() for faster
type checking and avoids calling Object.keys() when the headers are
stored as a 2D array instead of a plain object.
PR-URL: https://github.com/nodejs/node/pull/6533
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Since at least V8 5.4, using function.bind() is now fast enough to
use to avoid recompiling/reoptimizing the same anonymous functions.
These changes especially impact http servers.
PR-URL: https://github.com/nodejs/node/pull/6533
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
The new table-based lookups perform significantly better for the
common cases (checking latin1 characters).
PR-URL: https://github.com/nodejs/node/pull/6533
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit takes advantage of the performance improvements V8 has
made to function.bind() in V8 5.4 and uses it to avoid constant
recompilation/reoptimization of the wrapper closure used in once().
This change results in ~27% performance increase for once().
PR-URL: https://github.com/nodejs/node/pull/10445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
assertSize() is adjusted to be inlineable according to V8's default
function size limits when determining inlineability. This results in
up to 11% performance gains when allocating any kind of Buffer.
Avoid avoids use of in, resulting in ~50% improvement when creating
a Buffer from an array-like object.
PR-URL: https://github.com/nodejs/node/pull/10443
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Allow `fs.read`, `fs.write` and `fs.writeFile` to take
`Uint8Array` arguments.
PR-URL: https://github.com/nodejs/node/pull/10382
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Currently, around line 417 lib/url.js is truncating hostname and put the
rest of hostname to the path if hostname length after `.` is equal or
more than 63. This behavior is different from browser behavior. I
changed the code so that it doesn’t truncate.
I also added the test example which has more than 63 length in after
`.` in hostname in test url.
PR-URL: https://github.com/nodejs/node/pull/9292
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Extend `fs.realpathSync` to cache the results for paths that are not
symlinks in addition to caching symlink mappings.
PR-URL: https://github.com/nodejs/node/pull/10253
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.
PR-URL: https://github.com/nodejs/node/pull/10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is to be consistent with the other operators and helps
understanding the context when the code is grepped.
PR-URL: https://github.com/nodejs/node/pull/10213
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Allow all methods on `buffer` and `Buffer` to take `Uint8Array`
arguments where it makes sense. On the native side, there is
effectively no difference, and as a bonus the `isUint8Array`
check is faster than `instanceof Buffer`.
PR-URL: https://github.com/nodejs/node/pull/10236
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
This line `pool = null;` isn't needed and has
been around since the first iteration of streams.
I can't find a good reason for it to exist, it's
not more readable, nor does it seem to trick the
compiler into any optimizations.
PR-URL: https://github.com/nodejs/node/pull/10260
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Passphrase is now used whether keys are provided singly, in an array of
string/buffer, or an array of object, where it used to be ignored in
some argument combinations. Specifically, these now work as expected:
key: [encryptedPem],
passphrase: 'passphrase'
and
key: [{pem: encryptedPem}]
passphrase: 'passphrase'
and
key: [{pem: unencryptedPem}]
PR-URL: https://github.com/nodejs/node/pull/10294
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Changes disconnect() to return a refererence to the worker.
This will enable method chaining such as
worker.disconnect().once('disconnect', doThis);
PR-URL: https://github.com/nodejs/node/pull/10019
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
The fs function copyObject() had two arguments:
source and target. On the first line of the function it
assigned the target variable to:
arguments.length >= 2 ? target : {};
The function copyObject() was not called directly by
any test, but it is called in other fs functions. When it
was called it was only ever called with a single argument,
source. Thus I have removed the target argument and assigned
it to an empty object like it was being assigned to in the
original ternary operator.
PR-URL: https://github.com/nodejs/node/pull/10041
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Even though an Error object is passed to the callback when readFile()
fails due to toString() failing, it is a bit strange to still see
data passed as the second argument. This commit changes that and only
passes the Error object in that case.
PR-URL: https://github.com/nodejs/node/pull/9670
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In the string returned from URL.inspect there was an extra semicolon
at the end when showHidden === true. The semicolon has been
removed and a test for the inspect function has been added. The test
parses the returned string, validates all of the contained keys/values
and tests logic related to the showHidden option.
PR-URL: https://github.com/nodejs/node/pull/10231
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
- Always return the same error message(hopefully more informative)
for buffer length > kMaxLength and avoid getting into V8 C++ land
for unnecessary checks.
- Use accurate RegExp(reusable as `common.bufferMaxSizeMsg`)
in tests for this error.
- Separate related tests from test-buffer-alloc.
PR-URL: https://github.com/nodejs/node/pull/10152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Its confusing to have multiple names for the same thing, use
secureOptions consistently.
PR-URL: https://github.com/nodejs/node/pull/9800
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Either the options or the listener argument to tls.createServer() was
optional, but not both. This makes no sense, so align the argument
checking and documentation with net.createServer(), which accepts the
same option sequence, and which tls.createServer() is modelled on.
PR-URL: https://github.com/nodejs/node/pull/9800
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Fix the fast path for `buffer.fill()` with a single-character string.
The fast path only works for strings that are equivalent to a
single-byte buffer, but that condition was not checked properly
for the `utf8` or `utf16le` encodings and is always true for the
`latin1` encoding.
This change fixes these problems.
Fixes: https://github.com/nodejs/node/issues/9836
PR-URL: https://github.com/nodejs/node/pull/9837
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
- Add tests to check if the `originFor` implementation
for WHATWG url parsing is correnct.
- Fix `originFor` by including a base as argument
PR-URL: https://github.com/nodejs/node/pull/10021
Reviewed-By: James M Snell <jasnell@gmail.com>
We were transporting the heap statistics as uint32 values to JS land but
those wrap around for values > 4 GB. Use 64 bits floats instead, those
should last us a while.
Fixes: https://github.com/nodejs/node/issues/10185
PR-URL: https://github.com/nodejs/node/pull/10186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
We have a tacit rule that for multiline statements, the operator should
be placed before the linebreak. This commit commit fixes the few
violations of this rule in the code base.
This allows us to enable the corresponding ESLint rule.
PR-URL: https://github.com/nodejs/node/pull/10178
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
In order to prevent a memory leak when using keep alive, ensure that the
timeout listener for the request is removed when the response has ended.
PR-URL: https://github.com/nodejs/node/pull/9440
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Prior to this commit, it was possible to pass a truthy non-string
value as the HTTP method to the HTTP client, resulting in an
exception being thrown. This commit adds validation to the method.
PR-URL: https://github.com/nodejs/node/pull/10111
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Evan Torrie
James M Snell
Victor Felder
Brian White
Teddy Katz
Kai Cataldo
Benedikt Meurer
Joyee Cheung
Michaël Zasso
Anna Henningsen
cjihrig
Junshu Okamoto
Jeremy Yallop
Evan Lucas
Francis Gulotta
Sam Roberts
Sean Villars
hveldstra
Ethan Arrowood
Jay Brownlee
Ben Noordhuis
Karl Böhlmark
Luca Maraschi