From c74337e4a13aa24e13f2798f4cb6741b0d722557 Mon Sep 17 00:00:00 2001 From: Bojan Serafimov Date: Fri, 13 Jan 2023 02:27:02 -0500 Subject: [PATCH] try using im crate --- Cargo.lock | 68 ++++++++++++------- pageserver/Cargo.toml | 2 +- .../src/tenant/layer_map/layer_coverage.rs | 33 ++++----- 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 39855d010e..6116465c9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,15 +66,6 @@ dependencies = [ "backtrace", ] -[[package]] -name = "archery" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a8da9bc4c4053ee067669762bcaeea6e241841295a2b6c948312dad6ef4cc02" -dependencies = [ - "static_assertions", -] - [[package]] name = "asn1-rs" version = "0.5.1" @@ -615,6 +606,15 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "bitmaps" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "031043d04099746d8db04daf1fa424b2bc8bd69d92b25962dcde24da39ab64a2" +dependencies = [ + "typenum", +] + [[package]] name = "block-buffer" version = "0.10.3" @@ -1762,6 +1762,20 @@ dependencies = [ "unicode-normalization", ] +[[package]] +name = "im" +version = "15.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0acd33ff0285af998aaf9b57342af478078f53492322fafc47450e09397e0e9" +dependencies = [ + "bitmaps", + "rand_core", + "rand_xoshiro", + "sized-chunks", + "typenum", + "version_check", +] + [[package]] name = "indexmap" version = "1.9.2" @@ -2243,6 +2257,7 @@ dependencies = [ "humantime", "humantime-serde", "hyper", + "im", "itertools", "metrics", "nix", @@ -2260,7 +2275,6 @@ dependencies = [ "regex", "remote_storage", "reqwest", - "rpds", "rstar", "scopeguard", "serde", @@ -2767,6 +2781,15 @@ dependencies = [ "rand_core", ] +[[package]] +name = "rand_xoshiro" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f97cdb2a36ed4183de61b2f824cc45c9f1037f28afe0a322e9fff4c108b5aaa" +dependencies = [ + "rand_core", +] + [[package]] name = "rayon" version = "1.6.1" @@ -2936,15 +2959,6 @@ dependencies = [ "regex", ] -[[package]] -name = "rpds" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66262ea963eff99163e6b741fbc3417a52cc13074728c1047e9911789df9b000" -dependencies = [ - "archery", -] - [[package]] name = "rstar" version = "0.9.3" @@ -3454,6 +3468,16 @@ version = "0.3.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7bd3e3206899af3f8b12af284fafc038cc1dc2b41d1b89dd17297221c5d225de" +[[package]] +name = "sized-chunks" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16d69225bde7a69b235da73377861095455d298f2b970996eec25ddbb42b3d1e" +dependencies = [ + "bitmaps", + "typenum", +] + [[package]] name = "slab" version = "0.4.7" @@ -3500,12 +3524,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" -[[package]] -name = "static_assertions" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" - [[package]] name = "storage_broker" version = "0.1.0" diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index 395f450bb2..511ad35be6 100644 --- a/pageserver/Cargo.toml +++ b/pageserver/Cargo.toml @@ -67,7 +67,7 @@ storage_broker = { version = "0.1", path = "../storage_broker" } tenant_size_model = { path = "../libs/tenant_size_model" } utils = { path = "../libs/utils" } workspace_hack = { version = "0.1", path = "../workspace_hack" } -rpds = "0.12.0" +im = "15.1.0" reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] } [dev-dependencies] diff --git a/pageserver/src/tenant/layer_map/layer_coverage.rs b/pageserver/src/tenant/layer_map/layer_coverage.rs index 20af924058..5b6003d72e 100644 --- a/pageserver/src/tenant/layer_map/layer_coverage.rs +++ b/pageserver/src/tenant/layer_map/layer_coverage.rs @@ -1,9 +1,8 @@ use std::ops::Range; -// TODO the `im` crate has 20x more downloads and also has -// persistent/immutable BTree. It also runs a bit faster but -// results are not the same on some tests. -use rpds::RedBlackTreeMapSync; +// There's an alternative non-sync crate im-rc that's supposed to be 20% +// faster. It's not necessary now, but if extra perf is needed that's an option. +use im::OrdMap; /// Data structure that can efficiently: /// - find the latest layer by lsn.end at a given key @@ -19,11 +18,7 @@ pub struct LayerCoverage { /// We use an immutable/persistent tree so that we can keep historic /// versions of this coverage without cloning the whole thing and /// incurring quadratic memory cost. See HistoricLayerCoverage. - /// - /// We use the Sync version of the map because we want Self to - /// be Sync. Using nonsync might be faster, if we can work with - /// that. - nodes: RedBlackTreeMapSync>, + nodes: OrdMap>, } impl Default for LayerCoverage { @@ -35,7 +30,7 @@ impl Default for LayerCoverage { impl LayerCoverage { pub fn new() -> Self { Self { - nodes: RedBlackTreeMapSync::default(), + nodes: OrdMap::default(), } } @@ -48,7 +43,7 @@ impl LayerCoverage { Some((_, None)) => None, None => None, }; - self.nodes.insert_mut(key, value); + self.nodes.insert(key, value); } /// Insert a layer. @@ -88,10 +83,10 @@ impl LayerCoverage { to_remove.push(key.end); } for k in to_update { - self.nodes.insert_mut(k, Some((lsn.end, value.clone()))); + self.nodes.insert(k, Some((lsn.end, value.clone()))); } for k in to_remove { - self.nodes.remove_mut(&k); + self.nodes.remove(&k); } } @@ -99,10 +94,16 @@ impl LayerCoverage { /// /// Complexity: O(log N) pub fn query(&self, key: i128) -> Option { + + // This is cursed. Commenting out these lines makes the code + // correct, even though they don't do anything. + let k1 = self.nodes.get_prev(&key)?.0; + // let k2 = self.nodes.range(..=key).rev().next()?.0; + // assert_eq!(k1, k2); + self.nodes - .range(..=key) - .rev() - .next()? + // .get_prev(&key)? + .range(..=key).rev().next()? .1 .as_ref() .map(|(_, v)| v.clone())