From e5bd39a8b2ac960d6bbd3aeacc75765990888e74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 14:44:56 +0100 Subject: [PATCH 1/7] Tiny formatting change, because RustRover keeps redoing it. Having to revert on every commit is not fun. --- core/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index c0c579152..fc1f88fe2 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -50,7 +50,7 @@ regex-syntax = { version = "0.8.5", default-features = false, features = ["unico chrono = "0.4.38" julian_day_converter = "0.3.2" jsonb = { version = "0.4.4", optional = true } -indexmap = { version="2.2.6", features = ["serde"] } +indexmap = { version = "2.2.6", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } pest = { version = "2.0", optional = true } pest_derive = { version = "2.0", optional = true } From a17464ca5cd52391dd6334feb0255ff7ad1bd224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 14:47:29 +0100 Subject: [PATCH 2/7] cli: Add io_uring feature to match core --- cli/Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 193230b29..1886627e4 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -29,3 +29,6 @@ rustyline = "12.0.0" ctrlc = "3.4.4" csv = "1.3.1" miette = { version = "7.4.0", features = ["fancy"] } + +[features] +io_uring = ["limbo_core/io_uring"] From 2596e0800edfebbcc3d86d7389264f700226085a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 15:02:48 +0100 Subject: [PATCH 3/7] core: expose UnixIO apart from PlatformIO on any Unix, and expose UringIO on Linux with feature io_uring. cli: add a new argument to select I/O backend (more than one option only for Linux with io_uring feature). cli: make both Limbo::new() and Limbo::open_db() use get_io(), unifying parsing of database path and eliminating duplicated code. --- cli/app.rs | 112 +++++++++++++++++++++++++++++++++++++++---------- core/io/mod.rs | 4 ++ core/lib.rs | 5 ++- 3 files changed, 98 insertions(+), 23 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index 0b2f77641..fb44bc080 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -38,6 +38,52 @@ pub struct Opts { pub quiet: bool, #[clap(short, long, help = "Print commands before execution")] pub echo: bool, + #[clap( + default_value_t, + value_enum, + short, + long, + help = "Select I/O backend. The only other choice to 'syscall' is\n\ + \t'io-uring' when built for Linux with feature 'io_uring'\n" + )] + pub io: Io, +} + +#[derive(Clone)] +pub enum DbLocation { + Memory, + Path, +} + +#[derive(Clone, ValueEnum)] +pub enum Io { + Syscall, + #[cfg(all(target_os = "linux", feature = "io_uring"))] + IoUring, +} + +impl Default for Io { + /// Custom Default impl with cfg! macro, to provide compile-time default to Clap based on platform + /// The cfg! could be elided, but Clippy complains + /// The default value can still be overridden with the Clap argument + fn default() -> Self { + match cfg!(all(target_os = "linux", feature = "io_uring")) { + true => { + #[cfg(all(target_os = "linux", feature = "io_uring"))] + { + Io::IoUring + } + #[cfg(any( + not(target_os = "linux"), + all(target_os = "linux", not(feature = "io_uring")) + ))] + { + Io::Syscall + } + } + false => Io::Syscall, + } + } } #[derive(ValueEnum, Copy, Clone, Debug, PartialEq, Eq)] @@ -160,6 +206,7 @@ pub struct Settings { output_mode: OutputMode, echo: bool, is_stdout: bool, + io: Io, } impl From<&Opts> for Settings { @@ -174,6 +221,7 @@ impl From<&Opts> for Settings { .database .as_ref() .map_or(":memory:".to_string(), |p| p.to_string_lossy().to_string()), + io: opts.io.clone(), } } } @@ -207,7 +255,13 @@ impl Limbo { .as_ref() .map_or(":memory:".to_string(), |p| p.to_string_lossy().to_string()); - let io = get_io(&db_file)?; + let io_choice = opts.io.clone(); + let io = { + match db_file.as_str() { + ":memory:" => get_io(DbLocation::Memory, None)?, + _path => get_io(DbLocation::Path, Some(io_choice))?, + } + }; let db = Database::open_file(io.clone(), &db_file)?; let conn = db.connect(); let interrupt_count = Arc::new(AtomicUsize::new(0)); @@ -293,24 +347,17 @@ impl Limbo { fn open_db(&mut self, path: &str) -> anyhow::Result<()> { self.conn.close()?; - match path { - ":memory:" => { - let io: Arc = Arc::new(limbo_core::MemoryIO::new()?); - self.io = Arc::clone(&io); - let db = Database::open_file(self.io.clone(), path)?; - self.conn = db.connect(); - self.opts.db_file = ":memory:".to_string(); - Ok(()) + let io = { + match path { + ":memory:" => get_io(DbLocation::Memory, None)?, + _path => get_io(DbLocation::Path, Some(self.opts.io.clone()))?, } - path => { - let io: Arc = Arc::new(limbo_core::PlatformIO::new()?); - self.io = Arc::clone(&io); - let db = Database::open_file(self.io.clone(), path)?; - self.conn = db.connect(); - self.opts.db_file = path.to_string(); - Ok(()) - } - } + }; + self.io = Arc::clone(&io); + let db = Database::open_file(self.io.clone(), path)?; + self.conn = db.connect(); + self.opts.db_file = path.to_string(); + Ok(()) } fn set_output_file(&mut self, path: &str) -> Result<(), String> { @@ -740,10 +787,31 @@ fn get_writer(output: &str) -> Box { } } -fn get_io(db: &str) -> anyhow::Result> { - Ok(match db { - ":memory:" => Arc::new(limbo_core::MemoryIO::new()?), - _ => Arc::new(limbo_core::PlatformIO::new()?), +fn get_io( + db_location: DbLocation, + io_choice: Option, +) -> anyhow::Result> { + Ok(match db_location { + DbLocation::Memory => Arc::new(limbo_core::MemoryIO::new()?), + DbLocation::Path => { + match io_choice.unwrap() { + Io::Syscall => { + // We are building for Linux/macOS and syscall backend has been selected + #[cfg(target_family = "unix")] + { + Arc::new(limbo_core::UnixIO::new()?) + } + // We are not building for Linux/macOS and syscall backend has been selected + #[cfg(not(target_family = "unix"))] + { + Arc::new(limbo_core::PlatformIO::new()?) + } + } + // We are building for Linux and io_uring backend has been selected + #[cfg(all(target_os = "linux", feature = "io_uring"))] + Io::IoUring => Arc::new(limbo_core::UringIO::new()?), + } + } }) } diff --git a/core/io/mod.rs b/core/io/mod.rs index 7e910f4af..f88a5d554 100644 --- a/core/io/mod.rs +++ b/core/io/mod.rs @@ -166,11 +166,15 @@ impl Buffer { cfg_block! { #[cfg(all(target_os = "linux", feature = "io_uring"))] { mod io_uring; + pub use io_uring::UringIO; + mod unix; + pub use unix::UnixIO; pub use io_uring::UringIO as PlatformIO; } #[cfg(any(all(target_os = "linux",not(feature = "io_uring")), target_os = "macos"))] { mod unix; + pub use unix::UnixIO; pub use unix::UnixIO as PlatformIO; } diff --git a/core/lib.rs b/core/lib.rs index fd563cb19..a80fab83a 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -44,8 +44,11 @@ pub type Result = std::result::Result; use crate::translate::optimizer::optimize_plan; pub use io::OpenFlags; -#[cfg(feature = "fs")] pub use io::PlatformIO; +#[cfg(all(feature = "fs", target_family = "unix"))] +pub use io::UnixIO; +#[cfg(all(feature = "fs", target_os = "linux", feature = "io_uring"))] +pub use io::UringIO; pub use io::{Buffer, Completion, File, MemoryIO, WriteCompletion, IO}; pub use storage::buffer_pool::BufferPool; pub use storage::database::DatabaseStorage; From 5a45df84dbf50647093d46dd67062cb2ac320bdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 15:33:42 +0100 Subject: [PATCH 4/7] core: add debug line when an IO backend is created. A user can check that the correct one has been selected. --- core/io/generic.rs | 3 ++- core/io/io_uring.rs | 1 + core/io/memory.rs | 2 ++ core/io/unix.rs | 3 ++- core/io/windows.rs | 3 ++- 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/core/io/generic.rs b/core/io/generic.rs index 17f51d792..79bcde49c 100644 --- a/core/io/generic.rs +++ b/core/io/generic.rs @@ -1,5 +1,5 @@ use crate::{Completion, File, LimboError, OpenFlags, Result, IO}; -use log::trace; +use log::{debug, trace}; use std::cell::RefCell; use std::io::{Read, Seek, Write}; use std::rc::Rc; @@ -8,6 +8,7 @@ pub struct GenericIO {} impl GenericIO { pub fn new() -> Result { + debug!("Using IO backend 'generic'"); Ok(Self {}) } } diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs index 54a02ee61..3278d61b9 100644 --- a/core/io/io_uring.rs +++ b/core/io/io_uring.rs @@ -70,6 +70,7 @@ impl UringIO { }; MAX_IOVECS], next_iovec: 0, }; + debug!("Using IO backend 'io-uring'"); Ok(Self { inner: Rc::new(RefCell::new(inner)), }) diff --git a/core/io/memory.rs b/core/io/memory.rs index 46aa8834f..6fc960e13 100644 --- a/core/io/memory.rs +++ b/core/io/memory.rs @@ -1,6 +1,7 @@ use super::{Buffer, Completion, File, OpenFlags, IO}; use crate::Result; +use log::debug; use std::{ cell::{RefCell, RefMut}, collections::BTreeMap, @@ -20,6 +21,7 @@ type MemPage = Box<[u8; PAGE_SIZE]>; impl MemoryIO { #[allow(clippy::arc_with_non_send_sync)] pub fn new() -> Result> { + debug!("Using IO backend 'memory'"); Ok(Arc::new(Self { pages: RefCell::new(BTreeMap::new()), size: RefCell::new(0), diff --git a/core/io/unix.rs b/core/io/unix.rs index c86e05ee4..db0e85ab1 100644 --- a/core/io/unix.rs +++ b/core/io/unix.rs @@ -4,7 +4,7 @@ use crate::Result; use super::{Completion, File, OpenFlags, IO}; use libc::{c_short, fcntl, flock, F_SETLK}; -use log::trace; +use log::{debug, trace}; use polling::{Event, Events, Poller}; use rustix::fd::{AsFd, AsRawFd}; use rustix::fs::OpenOptionsExt; @@ -22,6 +22,7 @@ pub struct UnixIO { impl UnixIO { pub fn new() -> Result { + debug!("Using IO backend 'syscall'"); Ok(Self { poller: Rc::new(RefCell::new(Poller::new()?)), events: Rc::new(RefCell::new(Events::new())), diff --git a/core/io/windows.rs b/core/io/windows.rs index 0eea9e4da..50acfcb50 100644 --- a/core/io/windows.rs +++ b/core/io/windows.rs @@ -1,5 +1,5 @@ use crate::{Completion, File, LimboError, OpenFlags, Result, IO}; -use log::trace; +use log::{debug, trace}; use std::cell::RefCell; use std::io::{Read, Seek, Write}; use std::rc::Rc; @@ -8,6 +8,7 @@ pub struct WindowsIO {} impl WindowsIO { pub fn new() -> Result { + debug!("Using IO backend 'syscall'"); Ok(Self {}) } } From b6304147227b282a6eb9809658e8bbf3ba443bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 21:50:09 +0100 Subject: [PATCH 5/7] cli: implement Copy for DbLocation and Io, as suggested by [Preston](https://github.com/PThorpe92) --- cli/app.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index fb44bc080..af89f2ab0 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -49,13 +49,13 @@ pub struct Opts { pub io: Io, } -#[derive(Clone)] +#[derive(Copy, Clone)] pub enum DbLocation { Memory, Path, } -#[derive(Clone, ValueEnum)] +#[derive(Copy, Clone, ValueEnum)] pub enum Io { Syscall, #[cfg(all(target_os = "linux", feature = "io_uring"))] From 486389d6fffe54305fb014b4a6fa623c4a468c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 21:54:41 +0100 Subject: [PATCH 6/7] cli: remove calls to Io::clone() as it is now Copy --- cli/app.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index af89f2ab0..2a05a854f 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -221,7 +221,7 @@ impl From<&Opts> for Settings { .database .as_ref() .map_or(":memory:".to_string(), |p| p.to_string_lossy().to_string()), - io: opts.io.clone(), + io: opts.io, } } } @@ -350,7 +350,7 @@ impl Limbo { let io = { match path { ":memory:" => get_io(DbLocation::Memory, None)?, - _path => get_io(DbLocation::Path, Some(self.opts.io.clone()))?, + _path => get_io(DbLocation::Path, Some(self.opts.io))?, } }; self.io = Arc::clone(&io); From aca38031a4efa3b609769f7f6179181aa378dd4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Sat, 11 Jan 2025 22:44:56 +0100 Subject: [PATCH 7/7] cli: pass Io without option to get_io(), since even when running in-memory we get a default Io from Clap. Also remove last pesky Io::clone() --- cli/app.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index 2a05a854f..f114b63a2 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -255,11 +255,10 @@ impl Limbo { .as_ref() .map_or(":memory:".to_string(), |p| p.to_string_lossy().to_string()); - let io_choice = opts.io.clone(); let io = { match db_file.as_str() { - ":memory:" => get_io(DbLocation::Memory, None)?, - _path => get_io(DbLocation::Path, Some(io_choice))?, + ":memory:" => get_io(DbLocation::Memory, opts.io)?, + _path => get_io(DbLocation::Path, opts.io)?, } }; let db = Database::open_file(io.clone(), &db_file)?; @@ -349,8 +348,8 @@ impl Limbo { self.conn.close()?; let io = { match path { - ":memory:" => get_io(DbLocation::Memory, None)?, - _path => get_io(DbLocation::Path, Some(self.opts.io))?, + ":memory:" => get_io(DbLocation::Memory, self.opts.io)?, + _path => get_io(DbLocation::Path, self.opts.io)?, } }; self.io = Arc::clone(&io); @@ -787,14 +786,11 @@ fn get_writer(output: &str) -> Box { } } -fn get_io( - db_location: DbLocation, - io_choice: Option, -) -> anyhow::Result> { +fn get_io(db_location: DbLocation, io_choice: Io) -> anyhow::Result> { Ok(match db_location { DbLocation::Memory => Arc::new(limbo_core::MemoryIO::new()?), DbLocation::Path => { - match io_choice.unwrap() { + match io_choice { Io::Syscall => { // We are building for Linux/macOS and syscall backend has been selected #[cfg(target_family = "unix")]