This commit improves url.parse() performance by 50-210%
with the existing url/url-parse benchmarks. Also, the
optimizations made in url.format() result in a 40%
increase in performance for url.resolve().
Some optimization strategies used in this commit include:
* Combining multiple searches on the same string into a
single loop
* Avoiding unnecessary string.split() and array.join()
* Minimizing creation of temporary strings
* Using a faster alternative to encodeURIComponent,
borrowed from the querystring module
PR-URL: https://github.com/nodejs/node/pull/4892
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Util.format is just a stateless function. Apply current console
as `this` is unnecessary.
PR-URL: https://github.com/nodejs/node/pull/5222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Previously tried to reassign a const.
PR-URL: https://github.com/nodejs/node/pull/5259
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Template string was syntactically incorrect. Copied Documentation code
would throw an error.
PR-URL: https://github.com/nodejs/node/pull/5240
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Fix missing links. Fix styling of printf() - once #5073 lands,
link to man page will be auto-generated. Fix several typos.
PR-URL: https://github.com/nodejs/node/pull/5225
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Before b212be08f6, input types were not checked in some path functions
and the inputs were passed directly to `regexp.exec()` which
implicitly converts its argument to a string.
This commit both removes the type checking added in b212be08f6 and
adds string coercion for those functions.
PR-URL: https://github.com/nodejs/node/pull/5244
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Before this commit, it was possible to push a partial character
to a readable stream where it was decoded as an empty string and
then added to the internal buffer. This caused the stream to not
emit any data, even when the rest of the character bytes were pushed
separately, because of a non-zero length check of the first chunk in
the internal buffer.
Fixes: https://github.com/nodejs/node/issues/5223
PR-URL: https://github.com/nodejs/node/pull/5226
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The role of `this.server` is now split between `this._server` and
`this.server`. Where the first one is used for counting active
connections of `net.Server`, and the latter one is just a public API for
users' consumption.
The reasoning for this is simple, `TLSSocket` instances wrap
`net.Socket` instances, thus both refer to the `net.Server` through the
`this.server` property. However, only one of them should be used for
`net.Server` connection count book-keeping, otherwise double-decrement
will happen on socket destruction.
Fix: #5083
PR-URL: https://github.com/nodejs/node/pull/5262
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit brings the process.send() signature into sync with
the child_process.send() documentation. Specifically, this commit
adds the callback argument and return type to the docs.
PR-URL: https://github.com/nodejs/node/pull/5284
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
fs watch currently needs special configuration on AIX and we
want to improve under https://github.com/nodejs/node/issues/5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.
test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX
PR-URL: https://github.com/nodejs/node/pull/5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This is an intermediate fix for an issue of accessing `TLSWrap` fields
after the parent handle was destroyed. While `close` listener cleans up
this field automatically, it can be done even earlier at the
`TLSWrap.close` call.
Proper fix is going to be submitted and landed after this one.
Fix: #5108
PR-URL: https://github.com/nodejs/node/pull/5168
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Currently, the repl allows multiline function declarations, strings, and
all sorts of niceties by catching the SyntaxErrors they issue and
ignoring them. However, the SyntaxError raised by multiline function
calls was not caught. This commit adds to the whitelist.
PR-URL: https://github.com/nodejs/node/pull/3823
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Add the following (seemingly non-controversial) ESLint rules:
* `constructor-super`: Verify calls of `super()` in constructors. Flags
situations that will result in runtime errors. Since we do not have 100%
code coverage in tests, linting for runtime errors is useful.
* `no-class-assign`: Flags cases where a class declaration is
overwritten via variable assignment later. It is difficult to think of a
situation where this is not an error, and easy to think of situations
(particularly in lengthy test files) where it could come up.
* `no-const-assign`: Assigning to a const after declaration is a runtime
error.
* `no-dupe-class-members`: Declare a class member twice, then only the
second one counts. This is analogous to redeclaring a variable.
* `no-this-before-super`: Using `this` or `super` in a derived class
before a call to `super()` is a `ReferenceError`
PR-URL: https://github.com/nodejs/node/pull/5210
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
If none of VS2013/VS2015 WiX installs are found on a builder
machine, skip node build and notify user about.
PR-URL: https://github.com/nodejs/node/pull/5220
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: https://github.com/nodejs/node/pull/5216
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
The comment refers to a property (called `_pendings`) that no longer
exists.
PR-URL: https://github.com/nodejs/node/pull/5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.
PR-URL: https://github.com/nodejs/node/pull/5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The value's type is unsigned so it will always be >= 0.
PR-URL: https://github.com/nodejs/node/pull/5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
process is an EventEmitter. There are operations that increment and
decrement the _eventsCount property of an EventEmitter.
process._eventsCount would previously get set to NaN. This change makes
process._eventsCount be calculated as expected.
PR-URL: https://github.com/nodejs/node/pull/5208
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Direct high-level discussion and questions about IP and trademark policy
to the TSC repository.
PR-URL: https://github.com/nodejs/node/pull/5227
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <r@va.gg>
- Made scrolling of the sidebar not affect the main page by using a combination
of overflow:hidden and overflow:scroll.
- Changed the scrollbar of the sidebar to be visible again for accessibilty
reasons and removed the indication gradient because it was causing too much
issues.
- In WebKit-based browsers, the scrollbar received custom styling making it
appear to be outside of the sidebar.
- The main content is no longer limited to 702px width, but now uses all
available space.
- Changed the background of the code blocks to a very similar, but neutral
color and made inline blocks the same color.
- Made inline code blocks inside italic sections not italic.
- Simplified styling of api_stability classes by introducing a common class
name for the stability levels.
- Fixed various issues related to the green hover background on links.
- Fixed code box overflow outside the main content area.
- Various minor tweaks to paddings and margins.
- Cleaned up numbers in the stylesheet, removing unnecessary units, decimals
and empty selectors.
PR-URL: https://github.com/nodejs/node/pull/5198
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit improves escape() performance by up to 15% with the
existing querystring-stringify benchmarks by reducing the number
of string concatentations. A potential deopt is also avoided by
making sure the index passed to charCodeAt() is within bounds.
PR-URL: https://github.com/nodejs/node/pull/5012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Before this, v8 would deopt when an out of bounds `inIndex` would get
passed to charCodeAt(). charCodeAt() returns NaN in such cases, so we
directly emulate that behavior as well.
Also, calls to charCodeAt() for constant strings have been replaced
by the raw character codes and parser state is now stored as an
integer instead of a string. Both of these provide a slight
performance increase.
PR-URL: https://github.com/nodejs/node/pull/5012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit improves parse() performance by ~20-200% with the various
querystring-parse benchmarks.
Some optimization strategies used in this commit include:
* Combining multiple searches (for '&', '=', and '+') on the same
string into a single loop
* Avoiding string.split()
* Minimizing creation of temporary strings
* Avoiding string decoding if no encoded bytes were found and the
default string decoder is being used
PR-URL: https://github.com/nodejs/node/pull/5012
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This change adds ESLint rules that meet two criteria:
* recommended by ESLint
* require no code changes
These rules are:
* `no-func-assign`: Disallow overwriting a function that was written
as a function declaration.
* `no-negated-in-lhs`: Disallow negated left operand of `in` operator.
It prevents `if(!a in b)` when `if(!(a in b))` is intended.
* `no-obj-calls`: Disallow global object function calls. It prevents
errors like `JSON()` and `Math()`.
to exercise the code in tests or whatever, it can sneak in.
* `use-isnan`: Prevents errors like `if (foo == NaN)`
* `no-octal`: Disallows confusing constructs like `var num = 071;`
* `no-delete-var`: Delete works on properties, not variables. Disallows
`delete foo`.
PR-URL: https://github.com/nodejs/node/pull/5188
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Make sure that calling MakeCallback multiple times within the same stack
does not allow the nextTickQueue or MicrotaskQueue to be processed in
any more than the first MakeCallback call.
Check that domains enter/exit poperly with multiple MakeCallback calls
and that errors are handled as expected
PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
After attempting to use ReThrow() and Reset() there were cases where
firing the domain's error handlers was not happening. Or in some cases
reentering MakeCallback would still cause the domain enter callback to
abort (because the error had not been Reset yet).
In order for the script to properly stop execution when a subsequent
call to MakeCallback throws it must not be located within a TryCatch.
PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Environment::TickInfo::last_threw() is no longer in use.
Also pass Isolate to few methods and fix whitespace alignment.
PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Add a scope that will allow MakeCallback to know whether or not it's
currently running. This will prevent nextTickQueue and the
MicrotaskQueue from being processed recursively. It is also required to
wrap the bootloading stage since it doesn't run within a MakeCallback.
Ref: https://github.com/nodejs/node-v0.x-archive/issues/9245
PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Implementations of error handling between node::MakeCallback() and
AsyncWrap::MakeCallback() do not return at the same point. Make both
executions work the same by moving the early return if there's a caught
exception just after the AsyncWrap post callback. Since the domain's
call stack is cleared on a caught exception there is no reason to call
its exit() callback.
Remove the SetVerbose() statement in the AsyncWrap pre/post callback
calls since it does not affect the callback call.
PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Prior to this change, the NodeBIO objects used to wrap fixed data had
`num` equal to -1. This caused them to return -1 and set the retry flags
when they ran out of data. Since the data is fixed, that's incorrect.
Instead they should return zero to signal EOF.
This change adds a new, static function, NodeBIO::NewFixed to create a
BIO that wraps fixed data and which returns zero when exhausted.
The practical impact of this is limited since most (all?) the parsing
functions that these BIOs get passed to consider any return value less
than one to be EOF and ignore the retry flags anyway.
PR-URL: https://github.com/nodejs/node/pull/5105
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Ported by exinfinitum from a PR by jasnell:
see https://github.com/nodejs/node-v0.x-archive/pull/14185
Allows the running of v8 tests on node's packaged v8 source code.
Note that the limited win32 support added by jasnell has NOT been ported,
and so these tests are currently UNIX ONLY.
Note that gclient depot tools
(see https://commondatastorage.googleapis.com/
chrome-infra-docs/flat/depot_tools/docs/html/
depot_tools_tutorial.html#_setting_up) and subversion are required
to run tests.
To perform tests, run the following commands:
make v8 DESTCPU=(ARCH)
make test-v8 DESTCPU=(ARCH)
where (ARCH) is your CPU architecture, e.g. x64, ia32.
DESTCPU MUST be specified for this to work properly.
Can also do tests on debug build by using "make test-v8 DESTCPU=(ARCH)
BUILDTYPE=Debug", or perform intl or benchmark tests via make
test-v8-intl or test-v8-benchmarks respectively.
Note that by default, quickcheck and TAP output are disabled, and i18n
is enabled. To activate these options, use options"QUICKCHECK=True" and
"ENABLE_V8_TAP=True" respectively.
Use "DISABLE_V8_I18N" to disable i18n.
Use V8_BUILD_OPTIONS to allow custom user-defined flags to be
appended onto "make v8".
Any tests performed after changes to the packaged v8 file will require
recompiling of v8, which can be done using "make v8 DESTCPU=(ARCH)".
Finally, two additional files necessary for one of the v8 tests have
been added to the v8 folder.
PR-URL: https://github.com/nodejs/node/pull/4704
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
This commit adds support for async createConnection()
implementations and is still backwards compatible with
synchronous createConnection() implementations.
This commit also makes the http client more friendly with
generic stream objects produced by createConnection() by
checking stream.writable instead of stream.destroyed as the
latter is currently a net.Socket-ism and not set by the core
stream implementations.
PR-URL: https://github.com/nodejs/node/pull/4638
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit prevents child process stdio streams from being
automatically flushed on child process exit/close if a 'readable'
event handler has been attached at the time of exit.
Without this, child process stdio data can be lost if the process
exits quickly and a `read()` (e.g. from a 'readable' handler)
hasn't had the chance to get called yet.
Fixes: https://github.com/nodejs/node/issues/5034
PR-URL: https://github.com/nodejs/node/pull/5036
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit reverts the const usage introduced by 68a6abc
because v8 currently cannot optimize functions that contain
these uses of const (unsupported phi use of const variable).
The performance difference in this case can be up to ~130%
for non-ascii/binary string encodings.
PR-URL: https://github.com/nodejs/node/pull/5134
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Brian White
Jackson Tian
Chinedu Francis Nwafili
Rafael Cepeda
Noah Rose
Alexander Makarenko
Fedor Indutny
cjihrig
Rich Trott
Ojas Shirekar
Michael Dawson
Zirak
Tsarevich Dmitry
Igor Klopov
James M Snell
Evan Lucas
Ben Noordhuis
Roman Reiss
Trevor Norris
Mikeal Rogers
Adam Langley
Bryon Leung