This is a reland of https://github.com/nodejs/node/pull/3165. The patch abates
the truncation of script filenames in the perf-event output produced by V8.
V8 commits:
Original: 03ef3cd004
Reland: 010897c16a
Original commit message:
improve perf_basic_prof filename reporting
The buffer used for appending filenames to the string printed to the
perf_basic_prof log was unnecessarily too small. Bump it up to be at least
kUtf8BufferSize.
Truncation of filenames makes it really hard to work with profiles gathered on
Node.js. Because of the way Node.js works, you can have node module dependencies
in deeply nested directories. The last thing you want when investigating a
performance problem is to have script names be truncated.
This patch is a stop-gap. Ideally, I want no truncation of the filename at all
and use a dynamically growing buffer. That would be a larger change, and I
wanted to have a quick fix available that can be back-ported to Node.js LTS
release.
R=yangguo@chromium.org,yurys@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1388543002
PR-URL: https://github.com/nodejs/node/pull/3520
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Tests normally use common.PORT to allow the user to select which port
number to listen on. Hardcoding the port number will cause parallel
instances of the test to fail.
PR-URL: https://github.com/nodejs/node/pull/3557
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
As it is, the comments are not handled properly in REPL. So, if the
comments have `'` or `"`, then they are treated as incomplete string
literals and the error is thrown in REPL.
This patch refactors the existing logic and groups everything in a
class.
Fixes: https://github.com/nodejs/node/issues/3421
PR-URL: https://github.com/nodejs/node/pull/3515
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Original commit message:
[heap] fix crash during the scavenge of ArrayBuffer
Scavenger should not attempt to visit ArrayBuffer's storage, it is a
user-supplied pointer that may have any alignment. Visiting it, may
result in a crash.
BUG=
R=jochen
Review URL: https://codereview.chromium.org/1406133003
Cr-Commit-Position: refs/heads/master@{#31611}
PR-URL: https://github.com/nodejs/node/pull/3549
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
As per the `prefer-const` eslint rule, few instances of `let` have been
identified to be better with `const`. This patch updates all those
instances.
Refer: https://github.com/nodejs/node/issues/3118
PR-URL: https://github.com/nodejs/node/pull/3152
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Description from: http://eslint.org/docs/rules/prefer-const.html
If a variable is never modified, using the `const` declaration is
better. `const` declaration tells readers, "this variable is never
modified," reducing cognitive load and improving maintainability.
Refer: https://github.com/nodejs/node/issues/3118
PR-URL: https://github.com/nodejs/node/pull/3152
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Before this commit, sending a SIGUSR1 at program exit could trigger a
hard to reproduce race condition where `v8::Debug::DebugBreak(isolate)`
got called when the isolate was in the process of being torn down.
A similar race condition is in theory possible when sending signals
to two threads simultaneously but I haven't been able to reproduce
that myself (and I tried, oh how I tried.)
This commit fixes the race condition by turning `node_isolate` into
a `std::atomic` and using it as an ad hoc synchronization primitive
in places where that is necessary.
A bare minimum std::atomic polyfill is added for OS X because Apple
wouldn't be Apple if things just worked out of the box.
PR-URL: https://github.com/nodejs/node/pull/3528
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The current implementation overwrites the prototype of the target
constructor. It is not allowed with ES2015 classes because the prototype
property is read only. Use Object.setPrototypeOf instead.
Fixes: https://github.com/nodejs/node/issues/3452
PR-URL: https://github.com/nodejs/node/pull/3455
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If the URL passed to `http{s}.request` or `http{s}.get` is not properly
parsable by `url.parse`, we fall back to use `localhost` and port 80.
This creates confusing error messages like in this question
http://stackoverflow.com/q/32675907/1903116.
This patch throws an error message, if `url.parse` fails to parse the
URL properly.
Previous Discussion: https://github.com/nodejs/node/pull/2966
PR-URL: https://github.com/nodejs/node/pull/2967
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This fix is to be consistent implementation with ALPN. Tow NPN
protocol data in the persistent memebers move to hidden variables in
the wrap object.
PR-URL: https://github.com/nodejs/node/pull/2564
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
ALPN is added to tls according to RFC7301, which supersedes NPN.
When the server receives both NPN and ALPN extensions from the client,
ALPN takes precedence over NPN and the server does not send NPN
extension to the client. alpnProtocol in TLSSocket always returns
false when no selected protocol exists by ALPN.
In https server, http/1.1 token is always set when no
options.ALPNProtocols exists.
PR-URL: https://github.com/nodejs/node/pull/2564
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
inherits is used in lib and tests but its functionality itself is not
tested yet.
PR-URL: https://github.com/nodejs/node/pull/3507
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
If the resulting buffer.toString() call in fs.read throws, catch the
error and pass it back in the callback.
This issue only presents itself when fs.read is called using the legacy
string interface:
fs.read(fd, length, position, encoding, callback)
PR-URL: https://github.com/nodejs/node/pull/3503
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The test sometimes fail on an assertion but no useful error message
was generated for debugging. Modify the test to generate useful
debugging message.
PR-URL: https://github.com/nodejs/node/pull/3501
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Currently there are many instances where assert.fail is directly passed
to a callback for error handling. Unfortunately this will swallow the
error as it is the third argument of assert.fail that sets the message
not the first.
This commit adds a new function to test/common.js that simply wraps
assert.fail and calls it with the provided message.
Tip of the hat to @trott for pointing me in the direction of this.
PR-URL: https://github.com/nodejs/node/pull/3453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
that the standard allows them to either return a unique pointer or a
nullptr for zero-sized allocation requests. Normalize by always using
a nullptr.
Fixes: https://github.com/nodejs/node/issues/3496
PR-URL: https://github.com/nodejs/node/pull/3499
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
This test is already partially disabled for several platforms with
the comment that the required info is not provided at the C++ level.
I'm adding AIX as and PPC BE linux as they currently fall into
the same category. We are working to see if we can change that
in v8 but it will be non-trivial if is possible at all so I don't
want to leave the CI with failing tests until that point.
PR-URL: https://github.com/nodejs/node/pull/3491
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This PR is the first step enabling support for native modules
for AIX. The main issue is that unlike linux where all
symbols within the Node executable are available to the shared
library for a native module (npm), on AIX the symbols must
be explicitly exported. In addition, when the shared library is
built it must be linked using a list of the available symbols.
This patch covers the changes need to:
1) Export the symbols when building the node executable
2) Generate the file listing the symbols that can be used when
building the shared library.
For AIX, it breaks the build process into 2 steps. The first builds
a static library and then generates a node.exp file which contains
the symbols from that library. The second builds the node executable
and uses the node.exp file to specify which symbols should be
exported. In addition, it save the node.exp file so that it can
later be used in the creation of the shared library when building
a native module.
The following additional steps will be required in dependent projects
to fully enable AIX for native modules and are being worked
separately:
- Updates to node-gyp to use node.exp when creating the
shared library for a native module
- Fixes to gyp related to copying files as covered in
https://codereview.chromium.org/1368133002/patch/1/10001
- Pulling in updated gyp versions to Node and node-gyp
- Pulling latest libuv
These changes were done to minimize the change to other platforms
by working within the existing structure to add the 2 step process
for AIX without changing the process for other platforms.
PR-URL: https://github.com/nodejs/node/pull/3114
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
* Add a simple example for buffer.concat
* Change grammar slightly.
Fixes: #3219
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/3255
Emitting 'close' before the history has flushed is somewhat incorrect
and rather confusing.
This also makes the 'close' event always asynchronous for consistency.
Refs: https://github.com/nodejs/node/pull/2356
PR-URL: https://github.com/nodejs/node/pull/3435
Reviewed By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Updated test-npm to use absolute paths for tmp/cache/prefix
PR-URL: https://github.com/nodejs/node/pull/3309
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
v8 is faster at setting object properties in JS than C++. Even when it
requires calling into JS from native code. Make
process._getActiveRequests() faster by doing this when populating the
array containing request objects.
Simple benchmark:
for (let i = 0; i < 22; i++)
fs.open(__filename, 'r', function() { });
let t = process.hrtime();
for (let i = 0; i < 1e6; i++)
process._getActiveRequests();
t = process.hrtime(t);
console.log((t[0] * 1e9 + t[1]) / 1e6);
Results between the two:
Previous: 4406 ns/op
Patched: 690 ns/op 5.4x faster
PR-URL: https://github.com/nodejs/node/pull/3375
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <ben@strongloop.com>
If a tab completion is attempted on an undefined reference inside of a
function, the REPL was exiting without reporting an error or anything
else. This change results in the REPL reporting the ReferenceError and
continuing.
Fixes: https://github.com/nodejs/node/issues/3346
PR-URL: https://github.com/nodejs/node/pull/3358
Reviewed-By: James M Snell <jasnell@gmail.com>
test-child-process-fork-regr-gh-2847 could fail depending
on timing and how messages were packed into tcp packets.
If all of the requests fit into one packet then the test
worked otherwise, otherwise errors could occur. This PR
modifies the test to be tolerant while still validating that
some of the connection can be made succesfully
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/3459
Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: https://github.com/nodejs/node/pull/3458
This paragraph conveys that detached child processes do not stay
running in the background in general. Instead clarify that this
refers to the parent process exiting before the detached child
process is complete.
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/3250
This adds an example of a semi-common promise pattern that could
result in false-positive 'unhandledRejection' events, to help motivate
why there is a dual 'rejectionHandled' event. I anticipate it being
helpful to users who encounter 'unhandledRejection' events that do not
stem from obvious typos such as the JSON.pasre example.
Also cleans up the promise rejection tracking sample. By using a Map
instead of array we can get O(1) deletion and actually record the errors.
And, fixed indentation and backtick usage there to align with the rest of
the document.
Reviewed-By: Stephan Belanger <admin@stephenbelanger.com>
Reviewed-By: Petka Antonov <petka.antonov@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/3438
Add Benchmarking and Post-Mortem workgroups as they
were missing.
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <ben@strongloop.com>
PR-URL: https://github.com/nodejs/node/pull/3450
For Node.js v5.0.0, due to upgrade to V8 4.6 which has an incompatible
ABI
PR-URL: https://github.com/nodejs/node/pull/3400
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Require the test setup to obtain an EMFILE error and not ENFILE as
ENFILE means there is a race condition with other processes that may
close files before `spawn()` is called by the test.
Fixes: https://github.com/nodejs/node/issues/2666
PR-URL: https://github.com/nodejs/node/pull/3430
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove the Watchdog class' dependency on Environment.
No functional changes, only code cleanup.
PR-URL: https://github.com/nodejs/node/pull/3274
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Upon creating a TLSSocket object, set the default isServer option to false
Updated tls docs and added test-tls-socket-default-options
PR-URL: https://github.com/nodejs/node/pull/2614
Reviewed-By: Fedor Indutny <fedor@indutny.com>
This test has not failed on armv7-wheezy in over 6 weeks. Let's remove its
"flaky" status.
Fixes: https://github.com/nodejs/node/issues/2672
Reviewed-By: Rod Vagg <r@va.gg>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/3420
Ali Ijaz Sheikh
Stefan Budeanu
Sakthipriyan Vairamani
Fedor Indutny
Rich Trott
ronkorving
Ben Noordhuis
Michaël Zasso
Trevor Norris
Rod Vagg
Shigeki Ohtsu
Evan Lucas
Junliang Yan
Myles Borins
Michael Dawson
Rebecca Turner
Martii
Jeremiah Senkpiel
Imran Iqbal
Kyle Smith
Domenic Denicola
Ido Ben-Yair
Glen Keane
Yuval Brik