From 87c3e55449320f2816bdbfafe18b4e5643152ece Mon Sep 17 00:00:00 2001 From: Bojan Serafimov Date: Thu, 12 Jan 2023 13:23:59 -0500 Subject: [PATCH] Add early return to get_difficulty_map for 10x speedup --- pageserver/benches/bench_layer_map.rs | 4 ++-- pageserver/src/tenant/layer_map.rs | 27 ++++++++++++++++++--------- pageserver/src/tenant/timeline.rs | 6 ++++-- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/pageserver/benches/bench_layer_map.rs b/pageserver/benches/bench_layer_map.rs index 7a9e2fa5e1..896411dc4a 100644 --- a/pageserver/benches/bench_layer_map.rs +++ b/pageserver/benches/bench_layer_map.rs @@ -177,7 +177,7 @@ fn bench_from_real_project(c: &mut Criterion) { println!("Finished bruteforce in {:?}", now.elapsed()); let now = Instant::now(); - let result_fast = layer_map.get_difficulty_map(latest_lsn, &partitioning); + let result_fast = layer_map.get_difficulty_map(latest_lsn, &partitioning, None); assert!(result_fast.len() == partitioning.parts.len()); println!("Finished fast in {:?}", now.elapsed()); @@ -207,7 +207,7 @@ fn bench_from_real_project(c: &mut Criterion) { }); group.bench_function("get_difficulty_map", |b| { b.iter(|| { - layer_map.get_difficulty_map(latest_lsn, &partitioning); + layer_map.get_difficulty_map(latest_lsn, &partitioning, Some(3)); }); }); group.finish(); diff --git a/pageserver/src/tenant/layer_map.rs b/pageserver/src/tenant/layer_map.rs index c25b40bb19..ecba3da651 100644 --- a/pageserver/src/tenant/layer_map.rs +++ b/pageserver/src/tenant/layer_map.rs @@ -294,16 +294,20 @@ where } /// Count the height of the tallest stack of deltas in this 2d region. + /// + /// If `limit` is provided we don't try to count above that number. + /// /// This number is used to compute the largest number of deltas that /// we'll need to visit for any page reconstruction in this region. /// We use this heuristic to decide whether to create an image layer. - pub fn count_deltas(&self, key: &Range, lsn: &Range) -> Result { + pub fn count_deltas( + &self, key: &Range, lsn: &Range, limit: Option) -> Result { // We get the delta coverage of the region, and for each part of the coverage // we recurse right underneath the delta. The recursion depth is limited by // the largest result this function could return, which is in practice between // 3 and 10 (since we usually try to create an image when the number gets larger). - if lsn.is_empty() || key.is_empty() { + if lsn.is_empty() || key.is_empty() || limit == Some(0) { return Ok(0); } @@ -328,7 +332,8 @@ where let kr = Key::from_i128(current_key)..Key::from_i128(change_key); let lr = lsn.start..val.get_lsn_range().start; if !kr.is_empty() { - let max_stacked_deltas_underneath = self.count_deltas(&kr, &lr)?; + let new_limit = limit.map(|l| l - 1); + let max_stacked_deltas_underneath = self.count_deltas(&kr, &lr, new_limit)?; max_stacked_deltas = std::cmp::max(max_stacked_deltas, 1 + max_stacked_deltas_underneath); } @@ -346,7 +351,8 @@ where let lr = lsn.start..val.get_lsn_range().start; if !kr.is_empty() { - let max_stacked_deltas_underneath = self.count_deltas(&kr, &lr)?; + let new_limit = limit.map(|l| l - 1); + let max_stacked_deltas_underneath = self.count_deltas(&kr, &lr, new_limit)?; max_stacked_deltas = std::cmp::max(max_stacked_deltas, 1 + max_stacked_deltas_underneath); } @@ -434,29 +440,32 @@ where /// For each part of a keyspace partitioning, return the maximum number of layers /// that would be needed for page reconstruction in that part at the given LSN. /// + /// If `limit` is provided we don't try to count above that number. + /// /// This method is used to decide where to create new image layers. Computing the /// result for the entire partitioning at once allows this function to be more /// efficient, and further optimization is possible by using iterators instead, /// to allow early return. /// /// TODO actually use this method instead of count_deltas. Currently we only use - /// it for benchmarks. It's a drop-in replacement, but it would be good to - /// add early return to this method first so we don't make perf worse. - pub fn get_difficulty_map(&self, lsn: Lsn, partitioning: &KeyPartitioning) -> Vec { + /// it for benchmarks. + pub fn get_difficulty_map( + &self, lsn: Lsn, partitioning: &KeyPartitioning, limit: Option) -> Vec { // TODO This is a naive implementation. Perf improvements to do: // 1. Instead of calling self.image_coverage and self.count_deltas, // iterate the image and delta coverage only once. - // 2. Implement early return when the difficulty exceeds a threshold. partitioning .parts .iter() .map(|part| { let mut difficulty = 0; for range in &part.ranges { + if limit == Some(difficulty) {break;} for (img_range, last_img) in self .image_coverage(range, lsn) .expect("why would this err?") { + if limit == Some(difficulty) {break;} let img_lsn = if let Some(last_img) = last_img { last_img.get_lsn_range().end } else { @@ -465,7 +474,7 @@ where if img_lsn < lsn { let num_deltas = self - .count_deltas(&img_range, &(img_lsn..lsn)) + .count_deltas(&img_range, &(img_lsn..lsn), limit) .expect("why would this err lol?"); difficulty = std::cmp::max(difficulty, num_deltas); } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index c2a89cd6bb..d9c8552894 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -2263,13 +2263,15 @@ impl Timeline { // are some delta layers *later* than current 'lsn', if more WAL was processed and flushed // after we read last_record_lsn, which is passed here in the 'lsn' argument. if img_lsn < lsn { - let num_deltas = layers.count_deltas(&img_range, &(img_lsn..lsn))?; + let threshold = self.get_image_creation_threshold(); + let num_deltas = layers.count_deltas( + &img_range, &(img_lsn..lsn), Some(threshold))?; debug!( "key range {}-{}, has {} deltas on this timeline in LSN range {}..{}", img_range.start, img_range.end, num_deltas, img_lsn, lsn ); - if num_deltas >= self.get_image_creation_threshold() { + if num_deltas >= threshold { return Ok(true); } }