perf: [MR-544] Do not open overlay files twice

dfinity · Feb 12, 2024 · b635892 · b635892
1 parent b96f2f1
commit b635892
Show file tree

Hide file tree

Showing 8 changed files with 319 additions and 304 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/rs/canister_sandbox/src/protocol/sbxsvc.rs b/rs/canister_sandbox/src/protocol/sbxsvc.rs
@@ -10,8 +10,7 @@ use ic_interfaces::execution_environment::HypervisorResult;
 use ic_replicated_state::{
     page_map::{
         CheckpointSerialization, MappingSerialization, OverlayFileSerialization,
-        OverlayIndicesSerialization, PageAllocatorSerialization, PageMapSerialization,
-        StorageSerialization,
+        PageAllocatorSerialization, PageMapSerialization, StorageSerialization,
     },
     Global, NumWasmPages,
 };
@@ -141,13 +140,6 @@ impl EnumerateInnerFileDescriptors for StorageSerialization {
 impl EnumerateInnerFileDescriptors for OverlayFileSerialization {
     fn enumerate_fds<'a>(&'a mut self, fds: &mut Vec<&'a mut std::os::unix::io::RawFd>) {
         self.mapping.enumerate_fds(fds);
-        self.index.enumerate_fds(fds);
-    }
-}
-
-impl EnumerateInnerFileDescriptors for OverlayIndicesSerialization {
-    fn enumerate_fds<'a>(&'a mut self, fds: &mut Vec<&'a mut std::os::unix::io::RawFd>) {
-        fds.push(&mut self.file_descriptor.fd);
     }
 }
 

diff --git a/rs/replicated_state/BUILD.bazel b/rs/replicated_state/BUILD.bazel
@@ -51,6 +51,7 @@ DEV_DEPENDENCIES = [
     "//rs/crypto/test_utils/keys",
     "//rs/criterion_time",
     "//rs/test_utilities",
+    "//rs/test_utilities/metrics",
     "//rs/test_utilities/time",
     "@crate_index//:assert_matches",
     "@crate_index//:ic-btc-test-utils",

diff --git a/rs/replicated_state/Cargo.toml b/rs/replicated_state/Cargo.toml
@@ -53,6 +53,7 @@ criterion = "0.5"
 criterion-time = { path = "../criterion_time" }
 ic-btc-test-utils = { git = "https://github.com/dfinity/bitcoin-canister", rev = "b1693619e3d4dbc00d8c79e9b6886e1db48b21f7" }
 ic-test-utilities = { path = "../test_utilities" }
+ic-test-utilities-metrics = { path = "../test_utilities/metrics" }
 ic-test-utilities-time = { path = "../test_utilities/time" }
 maplit = "1.0.2"
 serde_cbor = { workspace = true }

diff --git a/rs/replicated_state/src/page_map.rs b/rs/replicated_state/src/page_map.rs
@@ -16,8 +16,8 @@ pub use page_allocator::{
     PageDeltaSerialization, PageSerialization,
 };
 pub use storage::{
-    MergeCandidate, OverlayFileSerialization, OverlayIndicesSerialization, StorageLayout,
-    StorageSerialization, MAX_NUMBER_OF_FILES,
+    MergeCandidate, OverlayFileSerialization, StorageLayout, StorageSerialization,
+    MAX_NUMBER_OF_FILES,
 };
 use storage::{OverlayFile, OverlayVersion, Storage};
 

diff --git a/rs/replicated_state/src/page_map/checkpoint.rs b/rs/replicated_state/src/page_map/checkpoint.rs
@@ -109,6 +109,7 @@ impl Mapping {
         Mapping::new(file, serialized_mapping.file_len as usize, None)
     }
 
+    /// Returns the `PageBytes` read from bytes `[PAGE_SIZE * page_index..PAGE_SIZE * (page_index + 1))`
     pub(crate) fn get_page(&self, page_index: PageIndex) -> &PageBytes {
         let num_pages = self.mmap.len() / PAGE_SIZE;
         if page_index.get() < num_pages as u64 {
@@ -122,6 +123,11 @@ impl Mapping {
         }
     }
 
+    /// The entire mmap as a slice of bytes.
+    pub(crate) fn as_slice(&self) -> &[u8] {
+        self.mmap.as_slice()
+    }
+
     /// See the comments of `PageMap::get_checkpoint_memory_instructions()`.
     pub fn get_memory_instructions(&self) -> MemoryInstructions {
         let num_pages = (self.mmap.len() / PAGE_SIZE) as u64;
@@ -135,10 +141,6 @@ impl Mapping {
         }
     }
 
-    pub fn num_pages(&self) -> usize {
-        self.mmap.len() / PAGE_SIZE
-    }
-
     pub(crate) fn file_descriptor(&self) -> &FileDescriptor {
         &self.file_descriptor
     }
@@ -202,7 +204,7 @@ impl Checkpoint {
     /// checkpoint.
     pub fn num_pages(&self) -> usize {
         match self.mapping {
-            Some(ref mapping) => mapping.num_pages(),
+            Some(ref mapping) => mapping.as_slice().len() / PAGE_SIZE,
             None => 0,
         }
     }