| Age | Commit message (Collapse) | Author |
|---|
|
Simply always tell the caller how many bytes were written, rather
than letting them track it.
In the case of writing a string, also keep track of the bytes
written by the earlier `DoTryWrite()`.
Refs: https://github.com/nodejs/node/issues/19562
PR-URL: https://github.com/nodejs/node/pull/19551
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Move tracking of `socket.bytesWritten` to C++ land.
This makes it easier to provide this functionality for all
`StreamBase` instances, and in particular should keep working
when they have been 'consumed' in C++ in some way (e.g. for
the network sockets that are underlying to TLS or HTTP2 streams).
Also, this parallels `socket.bytesRead` a lot more now.
PR-URL: https://github.com/nodejs/node/pull/19551
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This allows V8 to avoid preparing a execution context
for the constructor, to give a (kinda) small but noticeable
perf gain.
Benchmarks (only this commit):
$ ./node benchmark/compare.js --new ./node --old ./node-master --filter net-c2s.js --set len=10 --set type=asc --runs 360 net | Rscript benchmark/compare.R
[01:15:27|% 100| 1/1 files | 720/720 runs | 1/1 configs]: Done
confidence improvement accuracy (*) (**) (***)
net/net-c2s.js dur=5 type='asc' len=10 *** 0.69 % ±0.31% ±0.41% ±0.53%
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Provide a way to create pipes between native `StreamBase` instances
that acts more directly than a `.pipe()` call would.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Add a `OnStreamWantsWrite()` event that allows streams to
ask for more input data if they want some.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
This enables accessing files using a more standard pattern.
Once some more refactoring has been performed on the other existing
`StreamBase` streams, this could also be used to implement `fs`
streams in a more standard manner.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Put `HandleScope`s and `Context::Scope`s where they are used,
and don’t create one for native stream callbacks automatically.
This is slightly less convenient but means that stream listeners
that don’t actually call back into JS don’t have to pay the
(small) cost of setting these up.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/19204
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
|
|
`async` and `bytes` are only interesting when the write
is coming from JS, and unnecessary otherwise.
Also, make all of the stream `Write*()` bindings use the same
code for setting these, and upgrade to the non-deprecated versions.
PR-URL: https://github.com/nodejs/node/pull/18963
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
Replace v8::Persistent with node::Persistent, a specialization that
resets the persistent handle on destruction. Prevents accidental
resource leaks when forgetting to call .Reset() manually.
I'm fairly confident this commit fixes a number of resource leaks that
have gone undiagnosed so far.
PR-URL: https://github.com/nodejs/node/pull/18656
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
Encapsulate stream requests more:
- `WriteWrap` and `ShutdownWrap` classes are now tailored to the
streams on which they are used. In particular, for most streams
these are now plain `AsyncWrap`s and do not carry the overhead
of unused libuv request data.
- Provide generic `Write()` and `Shutdown()` methods that wrap
around the actual implementations, and make *usage* of streams
easier, rather than implementing; for example, wrap objects
don’t need to be provided by callers anymore.
- Use `EmitAfterWrite()` and `EmitAfterShutdown()` handlers to
call the corresponding JS handlers, rather than always trying
to call them. This makes usage of streams by other C++ code
easier and leaner.
Also fix up some tests that were previously not actually testing
asynchronicity when the comments indicated that they would.
PR-URL: https://github.com/nodejs/node/pull/18676
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Instead of passing along the handle object, just set it as a
property on the stream handle object and let the read handler
grab it from there.
PR-URL: https://github.com/nodejs/node/pull/18334
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Instead of setting individual callbacks on streams and tracking
stream ownership through a boolean `consume_` flag, always have
one specific listener object in charge of a stream, and call
methods on that object rather than generic C-style callbacks.
PR-URL: https://github.com/nodejs/node/pull/18334
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Previously the getter would mutate the kDefaultTriggerAsncId value. This
refactor changes the setter to bind the current kDefaultTriggerAsncId to
a scope, such that the getter doesn't have to mutate its own value.
PR-URL: https://github.com/nodejs/node/pull/17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/17665
Fixes: https://github.com/nodejs/node/issues/17636
Refs: https://github.com/nodejs/node/pull/16482
Refs: https://github.com/nodejs/node/pull/16860
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Instead of having per-request callbacks, always call a callback
on the `StreamBase` instance itself for `WriteWrap` and `ShutdownWrap`.
This makes `WriteWrap` cleanup consistent for all stream classes,
since the after-write callback is always the same now.
If special handling is needed for writes that happen to a sub-class,
`AfterWrite` can be overridden by that class, rather than that
class providing its own callback (e.g. updating the write
queue size for libuv streams).
If special handling is needed for writes that happen on another
stream instance, the existing `after_write_cb()` callback
is used for that (e.g. custom code after writing to the
transport from a TLS stream).
As a nice bonus, this also makes `WriteWrap` and `ShutdownWrap`
instances slightly smaller.
PR-URL: https://github.com/nodejs/node/pull/17564
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This PR makes the prototype accessors added by StreamBase::AddMethods
nonenumerable and checks the signatures in the accessors so they
throw instead of raising assertions when called with incompatible
receivers. They could be enumerated when inspecting the prototype
with util.inspect or the inspector protocol.
PR-URL: https://github.com/nodejs/node/pull/16860
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
`WriteWrap` instances may contain extra storage space.
`self_size()` returns the size of the *entire* struct, member fields as
well as storage space, so it is not an accurate measure for the
storage space available.
Add a method `ExtraSize()` (like the existing `Extra()` for accessing
the storage memory) that yields the wanted value, and use it
in the HTTP2 impl to fix a crash.
PR-URL: https://github.com/nodejs/node/pull/16727
Refs: https://github.com/nodejs/node/pull/16669
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/16548
Fixes: https://github.com/nodejs/node/issues/16519
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
Reduce the size of wrap objects by moving a couple of accessors from the
instance template to the prototype template. They occupied one slot per
instance instead of one slot per class.
This commit fixes some instances of unwrapping twice since that code had
to be updated anyway to use `args.This()` instead of `args.Holder()`.
PR-URL: https://github.com/nodejs/node/pull/16482
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
PR-URL: https://github.com/nodejs/node/pull/15569
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Changes in the native code for the upcoming async_hooks module. These
have been separated to help with review and testing.
Changes include:
* Introduce an async id stack that tracks recursive calls into async
execution contexts. For performance reasons the id stack is held as a
double* and assigned to a Float64Array. If the stack grows too large
it is then placed in it's own stack and replaced with a new double*.
This should accommodate arbitrarily large stacks.
I'm not especially happy with the complexity involved with this async
id stack, but it's also the fastest and most full proof way of
handling it that I have found.
* Add helper functions in Environment and AsyncWrap to work with the
async id stack.
* Add AsyncWrap::Reset() to allow AsyncWrap instances that have been
placed in a resource pool, instead of being released, to be
reinitialized. AsyncWrap::AsyncWrap() also now uses Reset() for
initialization.
* AsyncWrap* parent no longer needs to be passed via the constructor.
* Introduce Environment::AsyncHooks class to contain the needed native
functionality. This includes the pointer to the async id stack, and
array of v8::Eternal<v8::String>'s that hold the names of all
providers, mechanisms for storing/retrieving the trigger id, etc.
* Introduce Environment::AsyncHooks::ExecScope as a way to track the
current id and trigger id of function execution via RAII.
* If the user passes --abort-on-uncaught-exception then instead of
throwing the application will print a stack trace and abort.
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
|
|
When node began using the OneByte API (f150d56) it also switched to
officially supporting ISO-8859-1. Though at the time no new encoding
string was introduced.
Introduce the new encoding string 'latin1' to be more explicit. The
previous 'binary' and documented as an alias to 'latin1'. While many
tests have switched to use 'latin1', there are still plenty that do both
'binary' and 'latin1' checks side-by-side to ensure there is no
regression.
PR-URL: https://github.com/nodejs/node/pull/7111
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
For consistency with the newly added src/base64.h header, check that
NODE_WANT_INTERNALS is defined and set in internal headers.
PR-URL: https://github.com/nodejs/node/pull/6948
Refs: https://github.com/nodejs/node/pull/6910
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
|
|
v8::Object::GetAlignedPointerFromInternalField() returns a random value
if Wrap() hasn't been run on the object handle. Causing v8 to abort if
certain getters are accessed. It's possible to access these getters and
functions during class construction through the AsyncWrap init()
callback, and also possible in a subset of those scenarios while running
the persistent handle visitor.
Mitigate this issue by manually setting the internal aligned pointer
field to nullptr in the BaseObject constructor and add necessary logic
to return appropriate values when nullptr is encountered.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
First cast the pointer to the child Base class before casting to the
parent class to make sure it returns the correct pointer.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
This will provide `bytesRead` data on consumed sockets.
Fix: #3021
PR-URL: https://github.com/nodejs/node/pull/6284
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
v8::Handle is deprecated: https://codereview.chromium.org/1224623004
PR-URL: https://github.com/nodejs/io.js/pull/2202
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Expose and use in TLSWrap an `v8::External` wrap of the
`StreamBase*` pointer instead of guessing the ancestor C++ class in
`node_wrap.h`.
Make use of `StreamBase::Callback` structure for storing/passing both
callback and context in a single object.
Introduce `GetObject()` for future user-land usage, when a child class
is not going to be inherited from AsyncWrap.
PR-URL: https://github.com/nodejs/node/pull/2351
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
|
|
Re-add the wrapper class id to AsyncWrap instances so they can be
tracked directly in a heapdump.
Previously the class id was given without setting the heap dump wrapper
class info provider. Causing a segfault when a heapdump was taken. This
has been added, and the label_ set to the given provider name so each
instance can be identified.
The id will not be set of the passed object has no internal field count.
As the class pointer cannot be retrieved from the object.
In order to properly report the allocated size of each class, the new
pure virtual method self_size() has been introduces.
PR-URL: https://github.com/nodejs/io.js/pull/1896
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
It's possible for an accessor or named interceptor to get called with
a different execution context than the one it lives in, see the test
case for an example using the debug API.
This commit fortifies against that by passing the environment as a
data property instead of looking it up through the current context.
Fixes: https://github.com/iojs/io.js/issues/1190 (again)
PR-URL: https://github.com/iojs/io.js/pull/1238
Reviewed-By: Fedor Indutny <fedor@indutny.com>
|
|
Encapsulate allocation/disposal of `WriteWrap` instances into the
`WriteWrap` class itself.
PR-URL: https://github.com/iojs/io.js/pull/1090
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
UV_TTY does not support `uv_shutdown()` so adding this method in
StreamBase will cause an `abort()` in C land.
Fix: https://github.com/iojs/io.js/issues/1068
PR-URL: https://github.com/iojs/io.js/pull/1073
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
Only TCP and JSStream do support `.writev()` on all platforms at the
moment. Ensure that it won't be enabled everywhere.
Fix: https://github.com/iojs/io.js/issues/995
PR-URL: https://github.com/iojs/io.js/pull/1008
Reviewed-by: Bert Belder <bertbelder@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
|
|
Move JS methods to the stream_base-inl.h and thus define them on each
use of `StreamBase::AddMethods`. Inline `AddMethods` itself, so that
there won't be any need in a static declaration in stream_base.cc.
NOTE: This basically allows using this API in user-land, though, some
polishing is required before releasing it.
PR-URL: https://github.com/iojs/io.js/pull/957
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
|
