experiment: io_uring support #16265

littledivy · 2022-10-13T17:05:09Z

This PR does initial integration for using io_uring in Deno. See #16232

Implements specialized tokio_iouring ops for writeFile and readFile. The tokio
current_thread runtime is managed by tokio_iouring::start. We want to continue
adding more ops specialized for iouring.

Additionally, all of io_uring functionality is behind the enable_iouring Cargo feature
flag on deno_runtime. This is so that we do not distrupt workflow for people working
on this repo.

To compile Deno with io_uring support:

cargo build --release --features "deno_runtime/enable_iouring"

`tokio_iouring`

It's early but has amazing integration with tokio's current_thread runtime. Blocked on the following tokio_iouring PRs:

See stuff that Deno needs (ugly code alert).

Is it the right choice for Deno? This is what we aim to find out in the coming days with the experimental compilation flag.

Benchmarks

4x improvement in await Deno.writeFile(file, new Uint8Array(1024 * 1024 * 10)):

divy@divy:~/deno$ target/release/deno run -A --unstable cli/bench/write_file.js # io_uring
time 433 ms rate 230
time 548 ms rate 182
time 540 ms rate 185
time 565 ms rate 176
^C

divy@divy:~/deno$ ~/.deno/bin/deno run -A --unstable cli/bench/write_file.js # main
time 2696 ms rate 37
time 2857 ms rate 35
time 2859 ms rate 34
time 2861 ms rate 34
^C

main writeFile flamegraph

io_uring writeFile flamegraph

Towards denoland#16232

littledivy · 2022-10-13T17:10:30Z

runtime/ops/asyncfs/iouring/mod.rs

+      .check(path, Some("Deno.readFile()"))?;
+  }
+
+  // TODO(@littledivy): Stat file to get the size.


this is WIP lol. resize below may explode allocations for large files. We should stat the file for size.

littledivy · 2022-10-13T17:13:32Z

serde_v8/Cargo.toml

@@ -12,12 +12,18 @@ description = "Rust to V8 serialization and deserialization"
 [lib]
 path = "lib.rs"

+[features]
+default = []
+# Requires a recent Linux kernel version (atleast )


Suggested change

# Requires a recent Linux kernel version (atleast )

# Requires a recent Linux kernel version (atleast 5.6; tested on >5.11.0)

I think 5.6 is safe to assume? Read op was added in 5.6. I'm on 5.15.0 and tokio_iouring seems to suggest >5.11.0

IORING_OP_READV and IORING_OP_READ_FIXED - what maps to the Readv and ProvideBuffers ops, if I read the the io_uring crate's source code correctly - were added in 5.1.

bnoordhuis · 2022-10-14T07:13:48Z

runtime/ops/asyncfs/iouring/mod.rs

+  options: Option<&OpenOptions>,
+  api_name: &str,
+) -> Result<(PathBuf, tokio_uring::fs::OpenOptions), AnyError> {
+  let path = Path::new(path).to_path_buf();


More of an aside but:

Suggested change

let path = Path::new(path).to_path_buf();

let path = PathBuf::from(path);

And maybe you don't even need to convert the &str to a Path or PathBuf?

bnoordhuis · 2022-10-14T07:20:51Z

runtime/ops/asyncfs/iouring/mod.rs

+        permissions.read.check(&path, Some(api_name))?;
+      }
+
+      if options.write || options.append {


Shouldn't this also check options.truncate and/or options.create_new? A mode like O_RDONLY|O_CREAT is valid (creates file, then opens it read-only) but I would consider that a write operation.

bnoordhuis · 2022-10-14T07:42:00Z

runtime/ops/asyncfs/iouring/mod.rs

+      .check(path, Some("Deno.readFile()"))?;
+  }
+
+  // TODO(@littledivy): Stat file to get the size.


Stat-then-read is not really sound because of TOCTOU. You should be prepared for the file to change size.

Stat size is simply a preallocation hint. We should stop reading only on EOF

bnoordhuis · 2022-10-14T09:08:16Z

runtime/ops/asyncfs/iouring/mod.rs

+    let mut offset: u64 = 0;
+    loop {
+      let (res, mut ret) =
+        file.read_at(buf.split_off(offset as usize), offset).await;


buf.split_off() is wildly inefficient for the offset > 0 case (creates new vec, copies data, leaves old vec's capacity unchanged, i.e., wasted.)

Perhaps a better way to handle this is to maintain a vec of vecs, then join them at the end. That's O(n) but that's better than O(n*m).

bnoordhuis · 2022-10-14T09:17:01Z

serde_v8/Cargo.toml

@@ -12,12 +12,18 @@ description = "Rust to V8 serialization and deserialization"
 [lib]
 path = "lib.rs"

+[features]
+default = []
+# Requires a recent Linux kernel version (atleast )


IORING_OP_READV and IORING_OP_READ_FIXED - what maps to the Readv and ProvideBuffers ops, if I read the the io_uring crate's source code correctly - were added in 5.1.

ry

This patch is cleanly implemented, but ties us ever more to Tokio. This work might need to be undone if we went in a Crimson or pollio direction.

espoal · 2022-10-15T14:29:46Z

question @littledivy : if I understood correctly here you use the unix syscall to open a file. Why didn't you use IORING_OP_OPENAT? that would save a syscall.

littledivy · 2022-10-15T14:43:18Z

@espoal that is a syncronous op for Deno.openSync.

littledivy · 2022-10-15T14:44:31Z

we are trying to optimize async I/O here. See runtime/ops/asyncfs/iouring/mod.rs

lino-levan · 2022-11-25T17:51:41Z

@littledivy are there any updates? All of the PRs you mentioned blocking this change are merged now. This would be big for linux perf.

KnorpelSenf · 2023-03-01T19:35:50Z

This patch is cleanly implemented, but ties us ever more to Tokio. This work might need to be undone if we went in a Crimson or pollio direction.

Is this the reason why this was abandoned?

aapoalas · 2023-03-25T11:19:43Z

@littledivy @ry Do we expect this PR to be taken up again at some point, or will it be abandoned altogether? ie. Should we close this?

littledivy · 2023-03-25T11:26:47Z

I still want to do this but I don't have time to prioritize this atm. PRs welcome

rektide · 2023-11-16T00:09:11Z

What are some take that would help this pr?

Each do the three blocking tokio-uring PR are merged now. Presumably updating this branch to use those would help?

bnoordhuis · 2023-11-16T08:04:35Z

As a data point: after adding io_uring support to libuv we've been receiving a veritable fountain of reports from people hitting kernel bugs, from 100% CPU consumption to file operations not working on CIFS mounts.

Our kernel blacklist is long and specific, and still growing longer and more specific; see e.g. libuv/libuv#4224 from just yesterday. I honestly can't really recommend io_uring right now.

espoal · 2023-11-16T08:11:21Z

@bnoordhuis io_uring is not thought to handle network mounted file systems.
I'm surprised of your report, because I built a cloud scale database on top of io_uring and I don't see those problems. Could you provide more sources? I will take time personally to debug and eventually propose a patch for the interface.

The ticket you mention do not refer to a bug, but to the fact that the interface of io_uring in kernel 6.1 improved dramatically, both in term of ergonomics and performance, hence why the community try to aim for that version at least.

bnoordhuis · 2023-11-16T09:14:46Z

@espoal search the nodejs/node and libuv/libuv issue trackers, you'll find plenty of issues, some still open.

I built a cloud scale database on top of io_uring and I don't see those problems

That probably means you're either targeting only really new kernels, or your user base is a fraction of node and libuv. Both projects have huge user bases and people hit (and report) the weirdest bugs. The war stories I could tell...

espoal · 2023-11-16T10:12:59Z

@bnoordhuis that's a very fair point. I also noticed a huge stability improvement simply upgrading from 6.0 to 6.1

Sorry if I came out as rude, but I felt that you were attacking my single favourite kernel interface 😄 thanks for taking the time to elaborate ❤️

bnoordhuis · 2023-11-16T10:14:46Z

No worries, didn't come across as rude.

feat: experimental io_uring support

0d70fc0

Towards denoland#16232

littledivy commented Oct 13, 2022

View reviewed changes

bnoordhuis reviewed Oct 14, 2022

View reviewed changes

ry reviewed Oct 14, 2022

View reviewed changes

littledivy closed this Mar 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

experiment: io_uring support #16265

experiment: io_uring support #16265

littledivy commented Oct 13, 2022 •

edited

Loading

littledivy Oct 13, 2022

littledivy Oct 13, 2022

bnoordhuis Oct 14, 2022

bnoordhuis Oct 14, 2022

bnoordhuis Oct 14, 2022

bnoordhuis Oct 14, 2022

littledivy Oct 14, 2022

bnoordhuis Oct 14, 2022

bnoordhuis Oct 14, 2022

ry left a comment •

edited

Loading

espoal commented Oct 15, 2022 •

edited

Loading

littledivy commented Oct 15, 2022

littledivy commented Oct 15, 2022

lino-levan commented Nov 25, 2022

KnorpelSenf commented Mar 1, 2023

aapoalas commented Mar 25, 2023

littledivy commented Mar 25, 2023

rektide commented Nov 16, 2023

bnoordhuis commented Nov 16, 2023

espoal commented Nov 16, 2023

bnoordhuis commented Nov 16, 2023 •

edited

Loading

espoal commented Nov 16, 2023 •

edited

Loading

bnoordhuis commented Nov 16, 2023

	# Requires a recent Linux kernel version (atleast )
	# Requires a recent Linux kernel version (atleast 5.6; tested on >5.11.0)

	let path = Path::new(path).to_path_buf();
	let path = PathBuf::from(path);

experiment: io_uring support #16265

experiment: io_uring support #16265

Conversation

littledivy commented Oct 13, 2022 • edited Loading

tokio_iouring

Benchmarks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ry left a comment • edited Loading

Choose a reason for hiding this comment

espoal commented Oct 15, 2022 • edited Loading

littledivy commented Oct 15, 2022

littledivy commented Oct 15, 2022

lino-levan commented Nov 25, 2022

KnorpelSenf commented Mar 1, 2023

aapoalas commented Mar 25, 2023

littledivy commented Mar 25, 2023

rektide commented Nov 16, 2023

bnoordhuis commented Nov 16, 2023

espoal commented Nov 16, 2023

bnoordhuis commented Nov 16, 2023 • edited Loading

espoal commented Nov 16, 2023 • edited Loading

bnoordhuis commented Nov 16, 2023

littledivy commented Oct 13, 2022 •

edited

Loading

`tokio_iouring`

ry left a comment •

edited

Loading

espoal commented Oct 15, 2022 •

edited

Loading

bnoordhuis commented Nov 16, 2023 •

edited

Loading

espoal commented Nov 16, 2023 •

edited

Loading