Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ext/fetch): include TCP/IP connection info in fetch error messages #1

Closed
wants to merge 76 commits into from

Conversation

magurotuna
Copy link
Owner

bartlomieju and others added 30 commits July 30, 2024 14:34
…d#24788)

This commit duplicates ops from "ext/fetch" to "ext/node" to
kick off a bigger rewrite of "node:http".

Most of duplication is temporary and will be removed as these
ops evolve.
)

Fixes denoland#24323

- Use a Buffer pool for `fromString`
- Implement fast call base64 writes
- Direct from string `create` method for each encoding op

```
$ deno bench -A bench.mjs # 1.45.1+fee4d3a
cpu: Apple M1 Pro
runtime: deno 1.45.1+fee4d3a (aarch64-apple-darwin)

benchmark                time (avg)             (min … max)       p75       p99      p999
----------------------------------------------------------- -----------------------------
Buffer.from base64      550 ns/iter     (490 ns … 1'265 ns)    572 ns    606 ns  1'265 ns
Buffer#write base64     285 ns/iter       (259 ns … 371 ns)    307 ns    347 ns    360 ns

$ ~/gh/deno/target/release/deno bench -A bench.mjs # this PR
cpu: Apple M1 Pro
runtime: deno dev (aarch64-apple-darwin)

benchmark                time (avg)             (min … max)       p75       p99      p999
----------------------------------------------------------- -----------------------------
Buffer.from base64      151 ns/iter       (145 ns … 770 ns)    148 ns    184 ns    648 ns
Buffer#write base64   62.58 ns/iter     (60.79 ns … 157 ns)  61.65 ns  75.79 ns    141 ns

$ node bench.mjs # v22.4.0
cpu: Apple M1 Pro
runtime: node v22.4.0 (arm64-darwin)

benchmark                time (avg)             (min … max)       p75       p99      p999
----------------------------------------------------------- -----------------------------
Buffer.from base64      163 ns/iter     (96.92 ns … 375 ns)  99.45 ns    127 ns    220 ns
Buffer#write base64   75.48 ns/iter     (74.97 ns … 134 ns)  75.17 ns  81.83 ns  96.84 ns
```
Fixes denoland#24756. Fixes
denoland#24796.

This also gets vitest working when using
[`--pool=forks`](https://vitest.dev/guide/improving-performance#pool)
(which is the default as of vitest 2.0). Ref
denoland#23882.

---

This PR resolves a handful of issues with child_process IPC. In
particular:

- We didn't support sending typed array views over IPC
- Opening an IPC channel resulted in the event loop never exiting
- Sending a `null` over IPC would terminate the channel
- There was some UB in the read implementation (transmuting an `&[u8]`
to `&mut [u8]`)
- The `send` method wasn't returning anything, so there was no way to
signal backpressure (this also resulted in the benchmark
`child_process_ipc.mjs` being misleading, as it tried to respect
backpressure. That gave node much worse results at larger message sizes,
and gave us much worse results at smaller message sizes).
- We weren't setting up the `channel` property on the `process` global
(or on the `ChildProcess` object), and also didn't have a way to
ref/unref the channel
- Calling `kill` multiple times (or disconnecting the channel, then
calling kill) would throw an error
- Node couldn't spawn a deno subprocess and communicate with it over IPC
The way `fs.watch` works is different in `node:fs/promises` than
`node:fs`. It has a different function signature and it returns an async
iterable instead, see
https://nodejs.org/api/fs.html#fspromiseswatchfilename-options

Fixes denoland#24661
Some perf gains in swc (I measured formatting and it was slightly
faster).

Includes:

* denoland/deno_graph#508
* denoland/eszip#193
- upgrade to v8 12.8
  - optimizes DataView bigint methods
  - fixes global interceptors
  - includes CPED methods for ALS
- fix global resolution
- makes global resolution consistent using host_defined_options.
originally a separate patch but due to the global interceptor bug it
needs to be included in this pr for all tests to pass.
Uses [sui](https://github.com/littledivy/sui) to inject metadata as a
custom section in the denort binary.

Metadata is stored as a Mach-O segment on macOS and PE `RT_RCDATA`
resource on Windows.

Fixes denoland#11154 
Fixes denoland#17753

```cpp
deno compile app.tsx

# on macOS
codesign --sign - ./app

# on Windows
signtool sign /fd SHA256 .\app.exe
```

---------

Signed-off-by: Divy Srivastava <[email protected]>
This commit fixes the panic from
denoland#24137.

I'm not sure if we want to hard error or maybe instead skip with a
warning and continue execution.
…and#24811)

Prior to this commit, you could return a `Response` created from a
string or Uint8Array multiple times.

Now you can't do that anymore.
I noticed
[`set_response_body`](https://github.com/nathanwhit/deno/blob/ce42f82b5a985e5f1482dff97a7268019a8e79ea/ext/http/service.rs#L439-L443)
was unexpectedly hot in profiles, with most of the time being spent in
`memmove`.

It turns out that `ResponseBytesInner` was _massive_ (5624 bytes), so
every time we moved a `ResponseBytesInner` (for instance in
`set_response_body`) we were doing a >5kb memmove, which adds up pretty
quickly.

This PR boxes the two larger variants (the compression streams),
shrinking `ResponseBytesInner` to a reasonable 48 bytes.

---
  Benchmarked with a simple hello world server:
```ts
// hello-server.ts
Deno.serve((_req) => {
  return new Response("Hello world");
});
// run with `deno run -A hello-server.ts`
// in separate terminal `wrk -d 10s http://127.0.0.1:8000`
```

Main:
```
Running 10s test @ http://127.0.0.1:8000/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    53.39us    9.53us   0.98ms   92.78%
    Req/Sec    86.57k     3.56k   91.58k    91.09%
  1739319 requests in 10.10s, 248.81MB read
Requests/sec: 172220.92
Transfer/sec:     24.64MB
```

This PR:
```
Running 10s test @ http://127.0.0.1:8000/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    45.44us    8.49us   0.91ms   90.04%
    Req/Sec   100.65k     2.26k  102.65k    96.53%
  2022296 requests in 10.10s, 289.29MB read
Requests/sec: 200226.20
Transfer/sec:     28.64MB
```

So a nice ~15% bump. (With response body compression, the gain is ~10%
for gzip and neutral for brotli)
This PR fixes various typos I spotted in the project.
ry and others added 20 commits August 6, 2024 09:56
rewrite vm implementation to increase compat.
vm.Module+importModuleDynamically callbacks should be added in a
followup.
This PR updates `deno run` to fallback to executing tasks when there is
no script with the specified name. If there are both script and a task
with the same name then `deno run` will prioritise executing the script.
This completely rewrites how we handle key material in ext/node. Changes
in this
PR:

- **Signing**
  - RSA
  - RSA-PSS 🆕
  - DSA 🆕
  - EC
  - ED25519 🆕
- **Verifying**
  - RSA
  - RSA-PSS 🆕
  - DSA 🆕
  - EC 🆕
  - ED25519 🆕
- **Private key import**
  - Passphrase encrypted private keys 🆕
  - RSA
    - PEM
    - DER (PKCS#1) 🆕
    - DER (PKCS#8) 🆕
  - RSA-PSS
    - PEM
    - DER (PKCS#1) 🆕
    - DER (PKCS#8) 🆕
  - DSA 🆕
  - EC
    - PEM
    - DER (SEC1) 🆕
    - DER (PKCS#8) 🆕
  - X25519 🆕
  - ED25519 🆕
  - DH
- **Public key import**
  - RSA
    - PEM
    - DER (PKCS#1) 🆕
    - DER (PKCS#8) 🆕
  - RSA-PSS 🆕
  - DSA 🆕
  - EC 🆕
  - X25519 🆕
  - ED25519 🆕
  - DH 🆕
- **Private key export**
  - RSA 🆕
  - DSA 🆕
  - EC 🆕
  - X25519 🆕
  - ED25519 🆕
  - DH 🆕
- **Public key export**
  - RSA
  - DSA 🆕
  - EC 🆕
  - X25519 🆕
  - ED25519 🆕
  - DH 🆕
- **Key pair generation**
  - Overhauled, but supported APIs unchanged

This PR adds a lot of new individual functionality. But most importantly
because
of the new key material representation, it is now trivial to add new
algorithms
(as shown by this PR).

Now, when adding a new algorithm, it is also widely supported - for
example
previously we supported ED25519 key pair generation, but we could not
import,
export, sign or verify with ED25519. We can now do all of those things.
Additionally some renames in preparation to support "LTS" and "RC"
channels.
@magurotuna magurotuna changed the title Magurotuna/tcp conn info on error 2 fix(ext/fetch): include TCP/IP connection info in fetch error messages Aug 7, 2024
bartlomieju and others added 7 commits August 7, 2024 09:16
…oland#24923)

If the "size hint" does not provide a value, fall back to using a
"Content-Length" header.
… properties (denoland#24914)

Fixed `GPUAdapter` bugs:

* `GPUAdapter#isFallbackAdapter` being `undefined`
* `GPUAdapter#info` throwing `TypeError`
  * introduced by denoland#24783
* `GPUAdapter#info` closing adapter resources
  * introduced by denoland#23752
This commit updates the output of "deno upgrade" subcommand.
This PR ensures that we forward a `rename` event in our file watcher.
The rust lib we use combines that with the `modify` event.

This fixes a compatibility issue with Node too, which sends the `rename`
event as well.

Fixes denoland#24880
…land#24910)

This commit improves error messages that `fetch` generates on failure.

Fixes denoland#24835
@magurotuna magurotuna changed the base branch from magurotuna/improve-fetch-error to main August 8, 2024 07:04
@magurotuna magurotuna closed this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.