Home Original page

quic: share UDP code with dgram by addaleax · Pull Request #165 · nodejs/quic

@addaleax changed the title quic: share UDP code with dgram [WIP] quic: share UDP code with dgram

Oct 9, 2019

@addaleax addaleax marked this pull request as ready for review

October 11, 2019 23:40

@jasnell jasnell changed the title [WIP] quic: share UDP code with dgram quic: share UDP code with dgram

Oct 14, 2019 
This improves dgram performance by avoiding unnecessary async
operations.

One issue with this commit is that it seems hard to actually create
conditions under which the fallback path to the async case is
actually taken, for all supported OS, so an internal CLI option
is used for testing that path.

Another caveat is that the lack of an async operation means
that there are slight timing differences (essentially `nextTick()`
rather than `setImmediate()` for the send callback).

PR-URL: nodejs/node#29832
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This allows keeping `BaseObjectPtr`s to `HandleWrap` instances.
Previously, the pointer kept the `HandleWrap` object alive, leaving
the Environment cleanup code that waits for the handle list to drain
in a busy loop, because only the `HandleWrap` destructor removed
the item from the list.
Allow using the handle more directly for I/O in other parts of
the codebase.
This simplifies the code quite a bit.
This means that we always need to use `DeleteFnPtr` to make sure
that the timer is actually cleaned up, but I guess that’s an
acceptable restriction for now.
Keeping the JS object alive is not enough, because the
per-Environment cleanup may destroy C++ objects even if
their JS counterparts are still reachable.

@addaleax 

Provide facilities to test `QUIC` sockets without an actual
underlying UDP handle for more resilient tests.

@addaleax

@addaleax

@addaleax

addaleax added a commit that referenced this pull request

Oct 16, 2019 
This allows keeping `BaseObjectPtr`s to `HandleWrap` instances.
Previously, the pointer kept the `HandleWrap` object alive, leaving
the Environment cleanup code that waits for the handle list to drain
in a busy loop, because only the `HandleWrap` destructor removed
the item from the list.

PR-URL: #165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

addaleax added a commit that referenced this pull request

Oct 16, 2019 
Allow using the handle more directly for I/O in other parts of
the codebase.

PR-URL: #165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
Provide facilities to test `QUIC` sockets without an actual
underlying UDP handle for more resilient tests.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
Allow using the handle more directly for I/O in other parts of
the codebase.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
This simplifies the code quite a bit.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
This means that we always need to use `DeleteFnPtr` to make sure
that the timer is actually cleaned up, but I guess that’s an
acceptable restriction for now.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
Keeping the JS object alive is not enough, because the
per-Environment cleanup may destroy C++ objects even if
their JS counterparts are still reachable.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

juanarbol pushed a commit to juanarbol/quic that referenced this pull request

Dec 17, 2019 
Provide facilities to test `QUIC` sockets without an actual
underlying UDP handle for more resilient tests.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

jasnell pushed a commit to jasnell/quic that referenced this pull request

Feb 3, 2020 
Allow using the handle more directly for I/O in other parts of
the codebase.

PR-URL: nodejs#165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

jasnell pushed a commit that referenced this pull request

Feb 13, 2020 
Allow using the handle more directly for I/O in other parts of
the codebase.

PR-URL: #165
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

jasnell pushed a commit to jasnell/node that referenced this pull request

Mar 2, 2020 
Allow using the handle more directly for I/O in other parts of
the codebase.

Originally landed in the QUIC repo

Original review metadata:

```
  PR-URL: nodejs/quic#165
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
```

Signed-off-by: James M Snell <jasnell@gmail.com>

jasnell pushed a commit to nodejs/node that referenced this pull request

Mar 3, 2020 
Allow using the handle more directly for I/O in other parts of
the codebase.

Originally landed in the QUIC repo

Original review metadata:

```
  PR-URL: nodejs/quic#165
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #31871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins pushed a commit to nodejs/node that referenced this pull request

Mar 4, 2020 
Allow using the handle more directly for I/O in other parts of
the codebase.

Originally landed in the QUIC repo

Original review metadata:

```
  PR-URL: nodejs/quic#165
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #31871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>