Skip to content

Commit b64a933

Browse files
committed
fix: bound is_chart_bar_cluster's cubic cost on dense rect clusters
bar_family (used by is_chart_bar_cluster to distinguish bar charts from cell-rect grids) iterates every rect as an anchor, rebuilds a same-breadth "family" per anchor, then runs a nested any/filter over that family plus a per-member full-page item scan — O(n^3) in the cluster size in the worst case. Profiling a real fixture (tests/fixtures/bits_pilani_feedback.pdf, a dense feedback-form PDF) with samply + macOS `sample` showed is_chart_bar_cluster consuming ~25% of total runtime, taking the file from a documented ~200ms to 43s. A dense form/checkbox-grid page clusters into hundreds of touching rects via cluster_rects, which is exactly the case that makes the O(n^3) term dominate. No real bar chart has hundreds of bars, so this adds MAX_CHART_CLUSTER_RECTS (300, mirroring the existing MAX_CLUSTER_RECTS pattern in this file) as an early bailout — clusters above that size are treated as "not chart-like" instead of running the expensive checks, which costs no detections against realistic PDFs. Also hoists the redundant per-item `it.page == page` filter out of the per-rect-checked closure since it's invariant for the whole call. Verified: the fixture drops from 43s to 4.5s (~9.5x); full cargo test suite (721 tests, including a new regression test asserting the bailout fires in <100ms on an oversized cluster) passes unchanged.
1 parent b3d2158 commit b64a933

1 file changed

Lines changed: 49 additions & 2 deletions

File tree

src/tables/detect_rects.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2006,17 +2006,37 @@ fn without_dominant_page_backgrounds(rects: &[(f32, f32, f32, f32)]) -> Vec<(f32
20062006
/// holding at most a single numeric data label each. Bar charts drawn as
20072007
/// filled rects otherwise read as cell rects and grid their axis labels
20082008
/// into a phantom table. The mirrored check catches horizontal bar charts.
2009+
/// Above this many rects in a cluster, the O(n²)-per-anchor cost inside
2010+
/// `bar_family` below (itself run once per anchor, so effectively O(n³))
2011+
/// becomes prohibitively expensive — a single dense form/checkbox-grid page
2012+
/// (hundreds of touching rects clustered by `cluster_rects`) was measured
2013+
/// taking tens of seconds here alone. No real bar chart has this many bars,
2014+
/// so treating oversized clusters as "not chart-like" costs no detections
2015+
/// against realistic PDFs while keeping worst-case cost bounded. Mirrors
2016+
/// `MAX_CLUSTER_RECTS` above.
2017+
const MAX_CHART_CLUSTER_RECTS: usize = 300;
2018+
20092019
fn is_chart_bar_cluster(
20102020
items: &[TextItem],
20112021
group_rects: &[(f32, f32, f32, f32)],
20122022
page: u32,
20132023
) -> bool {
2024+
if group_rects.len() > MAX_CHART_CLUSTER_RECTS {
2025+
return false;
2026+
}
2027+
2028+
// Hoisted out of `numeric_or_empty` so the `it.page == page` filter runs
2029+
// once per call to `is_chart_bar_cluster` instead of once per rect
2030+
// checked (every anchor's family can call `numeric_or_empty` for each of
2031+
// its members).
2032+
let page_items: Vec<&TextItem> = items.iter().filter(|it| it.page == page).collect();
2033+
20142034
let numeric_or_empty = |(rx, ry, rw, rh): (f32, f32, f32, f32)| {
2015-
let inside: Vec<&TextItem> = items
2035+
let inside: Vec<&&TextItem> = page_items
20162036
.iter()
20172037
.filter(|it| {
20182038
let cx = it.x + it.width / 2.0;
2019-
it.page == page && cx >= rx && cx <= rx + rw && it.y >= ry && it.y <= ry + rh
2039+
cx >= rx && cx <= rx + rw && it.y >= ry && it.y <= ry + rh
20202040
})
20212041
.collect();
20222042
// Any number of numeric data labels is chart-like; a single run of
@@ -4668,4 +4688,31 @@ mod tests {
46684688
table.rows.len()
46694689
);
46704690
}
4691+
4692+
#[test]
4693+
fn is_chart_bar_cluster_bails_out_above_max_size() {
4694+
// A cluster larger than MAX_CHART_CLUSTER_RECTS must short-circuit
4695+
// to `false` instead of running bar_family's O(n^3) checks — this is
4696+
// the pathological case a dense form/checkbox-grid page hits (a real
4697+
// fixture measured 43s before this guard, ~4.5s after). Regression
4698+
// guard for the perf fix, independent of the exact heuristic result.
4699+
let oversized: Vec<(f32, f32, f32, f32)> = (0..MAX_CHART_CLUSTER_RECTS + 1)
4700+
.map(|i| (i as f32 * 12.0, 0.0, 10.0, 20.0))
4701+
.collect();
4702+
let items: Vec<TextItem> = Vec::new();
4703+
4704+
let start = std::time::Instant::now();
4705+
let result = is_chart_bar_cluster(&items, &oversized, 1);
4706+
let elapsed = start.elapsed();
4707+
4708+
assert!(
4709+
!result,
4710+
"oversized cluster must not be classified as a chart"
4711+
);
4712+
assert!(
4713+
elapsed.as_millis() < 100,
4714+
"oversized cluster should bail out immediately, took {:?}",
4715+
elapsed
4716+
);
4717+
}
46714718
}

0 commit comments

Comments
 (0)