Skip to content

Commit

Permalink
Replace std::ptr::copy() with copy_slice()
Browse files Browse the repository at this point in the history
Replace std::ptr::copy() with copy_slice(). With this change applied,
most accesses to guest memory are wrapped by copy_slice() to avoid
mixing read/write_volatile() and memcpy(). The file related interfaces,
read_from()/read_exact_from()/write_to()/write_all_to(), may still
use normal memory operations.

The change causes more than 50% performance drop of
copy_to_volatile_slice() when copying 10K data buffer. The performance
drop is caused by non-optimal implemantation of copy_slice(), which
should be tracked as a dedicate issue.

Benchmarking VolatileSlice::copy_to_volatile_slice:
	Collecting 200 samples in estimated 10.000 s
	VolatileSlice::copy_to_volatile_slice
        	time:   [684.24 ns 687.08 ns 691.81 ns]
                change: [+134.93% +136.04% +137.32%] (p = 0.00 < 0.05)
                Performance has regressed.
Found 11 outliers among 200 measurements (5.50%)
  1 (0.50%) low mild
  4 (2.00%) high mild
  6 (3.00%) high severe

Fixes: rust-vmm#111

Signed-off-by: Liu Jiang <[email protected]>
  • Loading branch information
jiangliu committed Aug 25, 2020
1 parent efb73ca commit 38d2b3f
Showing 1 changed file with 6 additions and 15 deletions.
21 changes: 6 additions & 15 deletions src/volatile_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use std::fmt;
use std::io::{self, Read, Write};
use std::marker::PhantomData;
use std::mem::{align_of, size_of};
use std::ptr::copy;
use std::ptr::{read_volatile, write_volatile};
use std::result;
use std::slice::{from_raw_parts, from_raw_parts_mut};
Expand Down Expand Up @@ -400,12 +399,10 @@ impl<'a> VolatileSlice<'a> {
/// # }
/// ```
pub fn copy_to_volatile_slice(&self, slice: VolatileSlice) {
// Safe because the pointers are range-checked when the slices
// are created, and they never escape the VolatileSlices.
unsafe {
// Safe because the pointers are range-checked when the slices
// are created, and they never escape the VolatileSlices.
// FIXME: ... however, is it really okay to mix non-volatile
// operations such as copy with read_volatile and write_volatile?
copy(self.addr, slice.addr, min(self.size, slice.size));
copy_slice(slice.as_mut_slice(), self.as_slice());
}
}

Expand Down Expand Up @@ -1003,16 +1000,10 @@ impl<'a, T: ByteValued> VolatileArrayRef<'a, T> {
/// # }
/// ```
pub fn copy_to_volatile_slice(&self, slice: VolatileSlice) {
// Safe because the pointers are range-checked when the slices
// are created, and they never escape the VolatileSlices.
unsafe {
// Safe because the pointers are range-checked when the slices
// are created, and they never escape the VolatileSlices.
// FIXME: ... however, is it really okay to mix non-volatile
// operations such as copy with read_volatile and write_volatile?
copy(
self.addr,
slice.addr,
min(self.len() * self.element_size(), slice.size),
);
copy_slice(slice.as_mut_slice(), self.to_slice().as_slice());
}
}

Expand Down

0 comments on commit 38d2b3f

Please sign in to comment.