From ce5a997a666436c371aefe89e217a7c921b40d95 Mon Sep 17 00:00:00 2001 From: Justin Cappos Date: Fri, 10 May 2024 23:03:05 -0400 Subject: [PATCH 1/4] changning benchmarks to open files read/write as is needed. --- benches/fs_read_write.rs | 4 ++-- benches/fs_read_write_seek.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/benches/fs_read_write.rs b/benches/fs_read_write.rs index dcf73b911..0770af8c5 100644 --- a/benches/fs_read_write.rs +++ b/benches/fs_read_write.rs @@ -54,7 +54,7 @@ pub fn run_benchmark(c: &mut Criterion) { &String::from_utf8(vec![b'X'; *buflen]).expect("error building string"), ); - let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA); + let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_RDWR, S_IRWXA); // Let's see how fast various file system calls are group.bench_with_input( BenchmarkId::new("TF02:Lind write", buflen), @@ -94,7 +94,7 @@ pub fn run_benchmark(c: &mut Criterion) { let path = c_str.into_raw() as *const u8; unsafe { - fd = libc::open(path, O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA); + fd = libc::open(path, O_CREAT | O_TRUNC | O_RDWR, S_IRWXA); } let deststring = tests::str2cbuf( diff --git a/benches/fs_read_write_seek.rs b/benches/fs_read_write_seek.rs index bc7eec58c..e4f7f01c9 100644 --- a/benches/fs_read_write_seek.rs +++ b/benches/fs_read_write_seek.rs @@ -47,7 +47,7 @@ pub fn run_benchmark(c: &mut Criterion) { // Iterate for different buffer sizes... for buflen in [1, 64, 1024, 65536].iter() { - let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA); + let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_RDWR, S_IRWXA); let deststring = tests::str2cbuf( &String::from_utf8(vec![b'X'; *buflen]).expect("error building string"), @@ -83,7 +83,7 @@ pub fn run_benchmark(c: &mut Criterion) { let path = c_str.into_raw() as *const u8; unsafe { - fd = libc::open(path, O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA); + fd = libc::open(path, O_CREAT | O_TRUNC | O_RDWR, S_IRWXA); } let deststring = tests::str2cbuf( From 678ffb29918a8667d7aa0a33d18bf07056cb887b Mon Sep 17 00:00:00 2001 From: Justin Cappos Date: Sat, 11 May 2024 08:26:38 -0400 Subject: [PATCH 2/4] Memory bug! Should have fixed the benchmarks. --- benches/fs_open_close.rs | 6 ++-- benches/fs_read_write.rs | 54 +++++++++++++++++++++++++++++++---- benches/fs_read_write_seek.rs | 32 +++++++++++++++------ 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/benches/fs_open_close.rs b/benches/fs_open_close.rs index b0d2ec73d..269211022 100644 --- a/benches/fs_open_close.rs +++ b/benches/fs_open_close.rs @@ -43,7 +43,8 @@ pub fn run_benchmark(c: &mut Criterion) { group.bench_function("TF01: Lind open+close", |b| { b.iter(|| { let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA); - cage.close_syscall(fd); + assert!(fd>2); // Ensure we didn't get an error or an odd fd + assert_eq!(cage.close_syscall(fd),0); // close the file w/o error }) }); @@ -55,7 +56,8 @@ pub fn run_benchmark(c: &mut Criterion) { O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA, ); - libc::close(fd); + assert!(fd>2); // Ensure we didn't get an error or an odd fd + assert_eq!(libc::close(fd),0); // close the file w/o error }) }); group.finish(); diff --git a/benches/fs_read_write.rs b/benches/fs_read_write.rs index 0770af8c5..dc832790d 100644 --- a/benches/fs_read_write.rs +++ b/benches/fs_read_write.rs @@ -54,6 +54,12 @@ pub fn run_benchmark(c: &mut Criterion) { &String::from_utf8(vec![b'X'; *buflen]).expect("error building string"), ); + // The size of the buffer and the amount we expect to read and write. + // I need to type convert this because it's a usize by default. + // I'm lazily converting with as here because it's not feasible to + // test values where usize would overflow this. + let expected_retval = *buflen as i32; + let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_RDWR, S_IRWXA); // Let's see how fast various file system calls are group.bench_with_input( @@ -61,13 +67,19 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| { - let _ = cage.write_syscall(fd, deststring, *buflen); + assert_eq!(cage.write_syscall(fd, deststring, *buflen),expected_retval); }) }, ); + // I'll read the file length so I don't overrun this with my reads... + let file_length = cage.lseek_syscall(fd, 0, SEEK_CUR); + cage.lseek_syscall(fd, 0, SEEK_SET); + // My current position when reading... + let mut pos = 0; + let mut read_buffer = tests::sizecbuf(*buflen); group.bench_with_input( @@ -75,7 +87,15 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| { - cage.read_syscall(fd, read_buffer.as_mut_ptr(), *buflen); + // Track the file pointer so you can backtrack if you make + // it to the end of the file. This avoids having a bunch + // of garbage, 0 length reads skew the results... + pos += expected_retval; + if file_length <= pos { + cage.lseek_syscall(fd, 0, SEEK_SET); + pos = 0; + } + assert_eq!(cage.read_syscall(fd, read_buffer.as_mut_ptr(), *buflen),expected_retval); }) }, ); @@ -91,6 +111,14 @@ pub fn run_benchmark(c: &mut Criterion) { let fd: c_int; let c_str = CString::new("/tmp/foo").unwrap(); + // The size of the buffer and the amount we expect to read and write. + // I need to type convert this because it's a usize by default. + // I'm lazily converting with as here because it's not feasible to + // test values where usize would overflow this. + // NOTE: This has a different type than Lind, which is i32. I think + // this is likely okay. + let expected_retval = *buflen as isize; + let path = c_str.into_raw() as *const u8; unsafe { @@ -107,15 +135,23 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| unsafe { - let _ = libc::write(fd, deststring as *const c_void, *buflen); + assert_eq!(libc::write(fd, deststring as *const c_void, *buflen),expected_retval); }) }, ); + // I'll read the file length so I don't overrun this with my reads... + let file_length:isize; unsafe { + file_length = libc::lseek(fd, 0, SEEK_CUR) as isize; + + // reset the file position libc::lseek(fd, 0, SEEK_SET); } + // My current position when reading... + let mut pos = 0; + let mut read_buffer = tests::sizecbuf(*buflen); // For comparison let's time the native OS... @@ -124,7 +160,15 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| unsafe { - libc::read(fd, read_buffer.as_mut_ptr() as *mut c_void, *buflen); + // Track the file pointer so you can backtrack if you make + // it to the end of the file. This avoids having a bunch + // of garbage, 0 length reads skew the results... + pos += expected_retval; + if file_length <= pos { + libc::lseek(fd, 0, SEEK_SET); + pos = 0; + } + assert_eq!(libc::read(fd, read_buffer.as_mut_ptr() as *mut c_void, *buflen),expected_retval); }) }, ); @@ -141,7 +185,7 @@ pub fn run_benchmark(c: &mut Criterion) { rustposix::safeposix::dispatcher::lindrustfinalize(); } -criterion_group!(name=benches; +criterion_group!(name=benches; // Add the global settings here so we don't type it everywhere config=global_criterion_settings::get_criterion(); targets=run_benchmark); diff --git a/benches/fs_read_write_seek.rs b/benches/fs_read_write_seek.rs index e4f7f01c9..1f638a161 100644 --- a/benches/fs_read_write_seek.rs +++ b/benches/fs_read_write_seek.rs @@ -53,6 +53,12 @@ pub fn run_benchmark(c: &mut Criterion) { &String::from_utf8(vec![b'X'; *buflen]).expect("error building string"), ); + // The size of the buffer and the amount we expect to read and write. + // I need to type convert this because it's a usize by default. + // I'm lazily converting with as here because it's not feasible to + // test values where usize would overflow this. + let expected_retval = *buflen as i32; + let read_buffer = tests::sizecbuf(*buflen).as_mut_ptr(); // Let's see how fast various file system calls are group.bench_with_input( @@ -60,10 +66,10 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| { - let _ = cage.write_syscall(fd, deststring, *buflen); - cage.lseek_syscall(fd, 0, SEEK_SET); - cage.read_syscall(fd, read_buffer, *buflen); - cage.lseek_syscall(fd, 0, SEEK_SET); + assert_eq!(cage.write_syscall(fd, deststring, *buflen),expected_retval); + assert_eq!(cage.lseek_syscall(fd, 0, SEEK_SET),0); + assert_eq!(cage.read_syscall(fd, read_buffer, *buflen),expected_retval); + assert_eq!(cage.lseek_syscall(fd, 0, SEEK_SET),0); }) }, ); @@ -92,16 +98,24 @@ pub fn run_benchmark(c: &mut Criterion) { let read_buffer = tests::sizecbuf(*buflen).as_mut_ptr(); + // The size of the buffer and the amount we expect to read and write. + // I need to type convert this because it's a usize by default. + // I'm lazily converting with as here because it's not feasible to + // test values where usize would overflow this. + // NOTE: This has a different type than Lind, which is i32. I think + // this is likely okay. + let expected_retval = *buflen as isize; + // For comparison let's time the native OS... group.bench_with_input( BenchmarkId::new("TF03:Native write+read+lseek", buflen), buflen, |b, buflen| { b.iter(|| unsafe { - let _ = libc::write(fd, deststring as *const c_void, *buflen); - libc::lseek(fd, 0, SEEK_SET); - libc::read(fd, read_buffer as *mut c_void, *buflen); - libc::lseek(fd, 0, SEEK_SET); + assert_eq!(libc::write(fd, deststring as *const c_void, *buflen),expected_retval); + assert_eq!(libc::lseek(fd, 0, SEEK_SET),0); + assert_eq!(libc::read(fd, read_buffer as *mut c_void, *buflen),expected_retval); + assert_eq!(libc::lseek(fd, 0, SEEK_SET),0); }) }, ); @@ -118,7 +132,7 @@ pub fn run_benchmark(c: &mut Criterion) { rustposix::safeposix::dispatcher::lindrustfinalize(); } -criterion_group!(name=benches; +criterion_group!(name=benches; // Add the global settings here so we don't type it everywhere config=global_criterion_settings::get_criterion(); targets=run_benchmark); From 83d92d799ef81c94df48208f3f8e60cbaa2a27c7 Mon Sep 17 00:00:00 2001 From: Justin Cappos Date: Sat, 11 May 2024 10:09:26 -0400 Subject: [PATCH 3/4] fixes for #242 --- benches/fs_open_close.rs | 8 ++--- benches/fs_read_write.rs | 58 +++++++++++++++++++++++++++-------- benches/fs_read_write_seek.rs | 22 ++++++++----- 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/benches/fs_open_close.rs b/benches/fs_open_close.rs index 269211022..d000dbf9e 100644 --- a/benches/fs_open_close.rs +++ b/benches/fs_open_close.rs @@ -43,8 +43,8 @@ pub fn run_benchmark(c: &mut Criterion) { group.bench_function("TF01: Lind open+close", |b| { b.iter(|| { let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA); - assert!(fd>2); // Ensure we didn't get an error or an odd fd - assert_eq!(cage.close_syscall(fd),0); // close the file w/o error + assert!(fd > 2); // Ensure we didn't get an error or an odd fd + assert_eq!(cage.close_syscall(fd), 0); // close the file w/o error }) }); @@ -56,8 +56,8 @@ pub fn run_benchmark(c: &mut Criterion) { O_CREAT | O_TRUNC | O_WRONLY, S_IRWXA, ); - assert!(fd>2); // Ensure we didn't get an error or an odd fd - assert_eq!(libc::close(fd),0); // close the file w/o error + assert!(fd > 2); // Ensure we didn't get an error or an odd fd + assert_eq!(libc::close(fd), 0); // close the file w/o error }) }); group.finish(); diff --git a/benches/fs_read_write.rs b/benches/fs_read_write.rs index dc832790d..1b1c02820 100644 --- a/benches/fs_read_write.rs +++ b/benches/fs_read_write.rs @@ -55,11 +55,22 @@ pub fn run_benchmark(c: &mut Criterion) { ); // The size of the buffer and the amount we expect to read and write. - // I need to type convert this because it's a usize by default. - // I'm lazily converting with as here because it's not feasible to - // test values where usize would overflow this. let expected_retval = *buflen as i32; + // My current position when writing... + let mut pos = 0; + + // Rather than track the file size, I will reset after a fixed amount + // of data is written. This is to avoid https://github.com/Lind-Project/safeposix-rust/issues/241 + // Once that bug is fixed, I should model the code used for Native + // and just track how much write has written, using this to know when + // to reset + const RESET_LENGTH: i32 = 1024 * 1024 * 4; // 4MB + + // Did I make it to the reset length? If not I will later abort so + // that I ensure I have enough to read. + let mut reached_reset_length: bool = false; + let fd = cage.open_syscall("foo", O_CREAT | O_TRUNC | O_RDWR, S_IRWXA); // Let's see how fast various file system calls are group.bench_with_input( @@ -67,18 +78,28 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| { - assert_eq!(cage.write_syscall(fd, deststring, *buflen),expected_retval); + pos += expected_retval; + // pos is the value we expect the pointer to be at AFTER + // the write, so we need < pos here to write until + // RESET_LENGTH + if RESET_LENGTH < pos { + cage.lseek_syscall(fd, 0, SEEK_SET); + pos = 0; + reached_reset_length = true; + } + assert_eq!(cage.write_syscall(fd, deststring, *buflen), expected_retval); }) }, ); - // I'll read the file length so I don't overrun this with my reads... - let file_length = cage.lseek_syscall(fd, 0, SEEK_CUR); + if !reached_reset_length { + panic!("Try decreasing RESET_LENGTH.\nOnly reached {}/{} bytes needed for read in Lind write.",pos,RESET_LENGTH); + } cage.lseek_syscall(fd, 0, SEEK_SET); // My current position when reading... - let mut pos = 0; + pos = 0; let mut read_buffer = tests::sizecbuf(*buflen); @@ -90,12 +111,17 @@ pub fn run_benchmark(c: &mut Criterion) { // Track the file pointer so you can backtrack if you make // it to the end of the file. This avoids having a bunch // of garbage, 0 length reads skew the results... + // We use <= here because we can have a read go to the + // expected EOF pos += expected_retval; - if file_length <= pos { + if RESET_LENGTH <= pos { cage.lseek_syscall(fd, 0, SEEK_SET); pos = 0; } - assert_eq!(cage.read_syscall(fd, read_buffer.as_mut_ptr(), *buflen),expected_retval); + assert_eq!( + cage.read_syscall(fd, read_buffer.as_mut_ptr(), *buflen), + expected_retval + ); }) }, ); @@ -135,19 +161,23 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| unsafe { - assert_eq!(libc::write(fd, deststring as *const c_void, *buflen),expected_retval); + assert_eq!( + libc::write(fd, deststring as *const c_void, *buflen), + expected_retval + ); }) }, ); // I'll read the file length so I don't overrun this with my reads... - let file_length:isize; + let file_length: isize; unsafe { file_length = libc::lseek(fd, 0, SEEK_CUR) as isize; // reset the file position libc::lseek(fd, 0, SEEK_SET); } + println!("FILE LENGTH: {}", file_length); // My current position when reading... let mut pos = 0; @@ -167,8 +197,12 @@ pub fn run_benchmark(c: &mut Criterion) { if file_length <= pos { libc::lseek(fd, 0, SEEK_SET); pos = 0; + print!("."); } - assert_eq!(libc::read(fd, read_buffer.as_mut_ptr() as *mut c_void, *buflen),expected_retval); + assert_eq!( + libc::read(fd, read_buffer.as_mut_ptr() as *mut c_void, *buflen), + expected_retval + ); }) }, ); diff --git a/benches/fs_read_write_seek.rs b/benches/fs_read_write_seek.rs index 1f638a161..676602d28 100644 --- a/benches/fs_read_write_seek.rs +++ b/benches/fs_read_write_seek.rs @@ -66,10 +66,10 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| { - assert_eq!(cage.write_syscall(fd, deststring, *buflen),expected_retval); - assert_eq!(cage.lseek_syscall(fd, 0, SEEK_SET),0); - assert_eq!(cage.read_syscall(fd, read_buffer, *buflen),expected_retval); - assert_eq!(cage.lseek_syscall(fd, 0, SEEK_SET),0); + assert_eq!(cage.write_syscall(fd, deststring, *buflen), expected_retval); + assert_eq!(cage.lseek_syscall(fd, 0, SEEK_SET), 0); + assert_eq!(cage.read_syscall(fd, read_buffer, *buflen), expected_retval); + assert_eq!(cage.lseek_syscall(fd, 0, SEEK_SET), 0); }) }, ); @@ -112,10 +112,16 @@ pub fn run_benchmark(c: &mut Criterion) { buflen, |b, buflen| { b.iter(|| unsafe { - assert_eq!(libc::write(fd, deststring as *const c_void, *buflen),expected_retval); - assert_eq!(libc::lseek(fd, 0, SEEK_SET),0); - assert_eq!(libc::read(fd, read_buffer as *mut c_void, *buflen),expected_retval); - assert_eq!(libc::lseek(fd, 0, SEEK_SET),0); + assert_eq!( + libc::write(fd, deststring as *const c_void, *buflen), + expected_retval + ); + assert_eq!(libc::lseek(fd, 0, SEEK_SET), 0); + assert_eq!( + libc::read(fd, read_buffer as *mut c_void, *buflen), + expected_retval + ); + assert_eq!(libc::lseek(fd, 0, SEEK_SET), 0); }) }, ); From e054d4a58335fe3436e270058ccfb4cd17babddf Mon Sep 17 00:00:00 2001 From: Justin Cappos Date: Sat, 11 May 2024 10:15:39 -0400 Subject: [PATCH 4/4] Remove extraneous print statements --- benches/fs_read_write.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/benches/fs_read_write.rs b/benches/fs_read_write.rs index 1b1c02820..0c57de4d3 100644 --- a/benches/fs_read_write.rs +++ b/benches/fs_read_write.rs @@ -177,7 +177,6 @@ pub fn run_benchmark(c: &mut Criterion) { // reset the file position libc::lseek(fd, 0, SEEK_SET); } - println!("FILE LENGTH: {}", file_length); // My current position when reading... let mut pos = 0; @@ -197,7 +196,6 @@ pub fn run_benchmark(c: &mut Criterion) { if file_length <= pos { libc::lseek(fd, 0, SEEK_SET); pos = 0; - print!("."); } assert_eq!( libc::read(fd, read_buffer.as_mut_ptr() as *mut c_void, *buflen),