Skip to content

Commit a415cf8

Browse files
authored
Improve many-entity performance by fixing redundant AnnotationContext lookups (also affects scene without annotation contexts) (#11445)
1 parent 7a1e668 commit a415cf8

File tree

18 files changed

+260
-104
lines changed

18 files changed

+260
-104
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10170,6 +10170,7 @@ dependencies = [
1017010170
"ndarray",
1017110171
"nohash-hasher",
1017210172
"parking_lot",
10173+
"rayon",
1017310174
"re_arrow_ui",
1017410175
"re_arrow_util",
1017510176
"re_byte_size",

crates/viewer/re_data_ui/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ pub fn annotations(
148148
) -> std::sync::Arc<re_viewer_context::Annotations> {
149149
re_tracing::profile_function!();
150150
let mut annotation_map = re_viewer_context::AnnotationMap::default();
151-
annotation_map.load(ctx, query, std::iter::once(entity_path));
151+
annotation_map.load(ctx, query);
152152
annotation_map.find(entity_path)
153153
}
154154

crates/viewer/re_test_viewport/src/lib.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,21 @@ impl TestContextExt for TestContext {
151151
ViewBlueprint::try_from_db(view_id, ctx.store_context.blueprint, ctx.blueprint_query)
152152
.expect("expected the view id to be known to the blueprint store");
153153

154-
let view_class = ctx
155-
.view_class_registry()
156-
.get_class_or_log_error(view_blueprint.class_identifier());
154+
let class_registry = ctx.view_class_registry();
155+
let class_identifier = view_blueprint.class_identifier();
156+
let view_class = class_registry.get_class_or_log_error(class_identifier);
157157

158158
let mut view_states = self.view_states.lock();
159159
let view_state = view_states.get_mut_or_create(view_id, view_class);
160160

161-
let (view_query, system_execution_output) =
162-
execute_systems_for_view(ctx, &view_blueprint, view_state);
161+
let context_system_static_exec_results = class_registry
162+
.run_static_context_systems_for_views(ctx, std::iter::once(class_identifier));
163+
let (view_query, system_execution_output) = execute_systems_for_view(
164+
ctx,
165+
&view_blueprint,
166+
view_state,
167+
&context_system_static_exec_results,
168+
);
163169

164170
view_class
165171
.ui(ctx, ui, view_state, &view_query, system_execution_output)

crates/viewer/re_view/src/annotation_scene_context.rs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
use std::sync::Arc;
2+
13
use re_viewer_context::{
2-
AnnotationMap, IdentifiedViewSystem, ViewContextSystem, ViewSystemIdentifier,
4+
AnnotationMap, IdentifiedViewSystem, ViewContextSystem, ViewContextSystemStaticExecResult,
5+
ViewSystemIdentifier,
36
};
47

58
#[derive(Default)]
6-
pub struct AnnotationSceneContext(pub AnnotationMap);
9+
pub struct AnnotationSceneContext(pub Arc<AnnotationMap>);
710

811
impl IdentifiedViewSystem for AnnotationSceneContext {
912
fn identifier() -> ViewSystemIdentifier {
@@ -12,19 +15,30 @@ impl IdentifiedViewSystem for AnnotationSceneContext {
1215
}
1316

1417
impl ViewContextSystem for AnnotationSceneContext {
18+
fn execute_static(
19+
ctx: &re_viewer_context::ViewerContext<'_>,
20+
) -> ViewContextSystemStaticExecResult {
21+
// Use static execution to load the annotation map for all entities.
22+
// Alternatively, we could do this only for visible ones per View but this is actually a lot more expensive to do
23+
// given that there's typically just one annotation map per recording anyways!
24+
let mut annotation_map = AnnotationMap::default();
25+
annotation_map.load(ctx, &ctx.current_query());
26+
27+
Box::new(Self(Arc::new(annotation_map)))
28+
}
29+
1530
fn execute(
1631
&mut self,
17-
ctx: &re_viewer_context::ViewContext<'_>,
18-
query: &re_viewer_context::ViewQuery<'_>,
32+
_ctx: &re_viewer_context::ViewContext<'_>,
33+
_query: &re_viewer_context::ViewQuery<'_>,
34+
static_execution_result: &ViewContextSystemStaticExecResult,
1935
) {
20-
re_tracing::profile_function!();
21-
// We create a list of *all* entities here, do not only iterate over those with annotation context.
22-
// TODO(andreas): But knowing ahead of time where we have annotation contexts could be used for optimization.
23-
self.0.load(
24-
ctx.viewer_ctx,
25-
&query.latest_at_query(),
26-
query.iter_all_entities(),
27-
);
36+
// Take over the static result to make it available.
37+
self.0 = static_execution_result
38+
.downcast_ref::<Self>()
39+
.expect("Unexpected static execution result type")
40+
.0
41+
.clone();
2842
}
2943

3044
fn as_any(&self) -> &dyn std::any::Any {

crates/viewer/re_view_map/src/map_view.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ impl ViewClass for MapView {
122122
system_registry.register_visualizer::<GeoLineStringsVisualizer>()?;
123123

124124
system_registry.register_context_system::<AnnotationSceneContext>()?;
125+
re_viewer_context::AnnotationContextStoreSubscriber::subscription_handle(); // Needed by `AnnotationSceneContext`
125126

126127
Ok(())
127128
}

crates/viewer/re_view_spatial/src/contexts/depth_offsets.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ impl ViewContextSystem for EntityDepthOffsets {
2929
&mut self,
3030
ctx: &re_viewer_context::ViewContext<'_>,
3131
query: &re_viewer_context::ViewQuery<'_>,
32+
_static_execution_result: &re_viewer_context::ViewContextSystemStaticExecResult,
3233
) {
3334
let mut entities_per_draw_order = BTreeMap::new();
3435
for (visualizer, draw_order_descriptor) in visualizers_processing_draw_order() {

crates/viewer/re_view_spatial/src/contexts/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ pub fn register_spatial_contexts(
2828
) -> Result<(), ViewClassRegistryError> {
2929
system_registry.register_context_system::<TransformTreeContext>()?;
3030
system_registry.register_context_system::<EntityDepthOffsets>()?;
31+
3132
system_registry.register_context_system::<AnnotationSceneContext>()?;
33+
re_viewer_context::AnnotationContextStoreSubscriber::subscription_handle(); // Needed by `AnnotationSceneContext`
34+
3235
Ok(())
3336
}

crates/viewer/re_view_spatial/src/contexts/transform_tree_context.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ use re_chunk_store::LatestAtQuery;
22
use re_log_types::{EntityPath, EntityPathHash};
33
use re_types::{archetypes, components::ImagePlaneDistance};
44
use re_view::DataResultQuery as _;
5-
use re_viewer_context::{DataResultTree, IdentifiedViewSystem, ViewContext, ViewContextSystem};
5+
use re_viewer_context::{
6+
DataResultTree, IdentifiedViewSystem, ViewContext, ViewContextSystem,
7+
ViewContextSystemStaticExecResult,
8+
};
69

710
use crate::{caches::TransformDatabaseStoreCache, visualizers::CamerasVisualizer};
811

@@ -20,6 +23,7 @@ impl ViewContextSystem for TransformTreeContext {
2023
&mut self,
2124
ctx: &re_viewer_context::ViewContext<'_>,
2225
query: &re_viewer_context::ViewQuery<'_>,
26+
_static_execution_result: &ViewContextSystemStaticExecResult,
2327
) {
2428
let recording = ctx.recording();
2529

crates/viewer/re_view_spatial/src/max_image_dimension_subscriber.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,7 @@ impl MaxImageDimensionsStoreSubscriber {
5555
move |subscriber: &Self| f(&subscriber.max_dimensions),
5656
)
5757
}
58-
}
5958

60-
impl MaxImageDimensionsStoreSubscriber {
6159
/// Accesses the global store subscriber.
6260
///
6361
/// Lazily registers the subscriber if it hasn't been registered yet.

crates/viewer/re_viewer_context/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ macaw.workspace = true
6868
ndarray.workspace = true
6969
nohash-hasher.workspace = true
7070
parking_lot = { workspace = true, features = ["serde"] }
71+
rayon.workspace = true
7172
serde.workspace = true
7273
slotmap.workspace = true
7374
smallvec.workspace = true

0 commit comments

Comments
 (0)