Skip to content

Commit d715f3e

Browse files
authored
Determine per-view overrides in parallel (-> faster many entities + many views on native) (#11439)
1 parent b39dd3c commit d715f3e

File tree

10 files changed

+141
-64
lines changed

10 files changed

+141
-64
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10062,6 +10062,7 @@ dependencies = [
1006210062
"js-sys",
1006310063
"parking_lot",
1006410064
"poll-promise",
10065+
"rayon",
1006510066
"re_analytics",
1006610067
"re_auth",
1006710068
"re_blueprint_tree",

crates/viewer/re_test_viewport/src/lib.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,16 @@ impl TestContextExt for TestContext {
8989
let view_blueprint = viewport_blueprint
9090
.view(view_id)
9191
.expect("view is known to exist");
92+
93+
let class_registry = ctx.view_class_registry();
9294
let class_identifier = view_blueprint.class_identifier();
95+
let class = class_registry.class(class_identifier).unwrap_or_else(|| panic!("The class '{class_identifier}' must be registered beforehand"));
9396

94-
let visualizable_entities = ctx
95-
.view_class_registry()
96-
.class(class_identifier)
97-
.unwrap_or_else(|| panic!("The class '{class_identifier}' must be registered beforehand"))
97+
let visualizable_entities = class
9898
.determine_visualizable_entities(
9999
ctx.maybe_visualizable_entities_per_visualizer,
100100
ctx.recording(),
101-
&ctx.view_class_registry()
101+
&class_registry
102102
.new_visualizer_collection(class_identifier),
103103
&view_blueprint.space_origin,
104104
);
@@ -109,14 +109,14 @@ impl TestContextExt for TestContext {
109109

110110
let mut data_query_result = view_blueprint.contents.execute_query(
111111
ctx.store_context,
112-
ctx.view_class_registry(),
112+
class_registry,
113113
ctx.blueprint_query,
114114
&visualizable_entities,
115115
);
116116

117117
let resolver = DataQueryPropertyResolver::new(
118118
view_blueprint,
119-
ctx.view_class_registry(),
119+
class_registry,
120120
ctx.maybe_visualizable_entities_per_visualizer,
121121
&visualizable_entities,
122122
&indicated_entities_per_visualizer,
@@ -126,9 +126,9 @@ impl TestContextExt for TestContext {
126126
ctx.store_context.blueprint,
127127
ctx.blueprint_query,
128128
ctx.rec_cfg.time_ctrl.read().timeline(),
129-
ctx.view_class_registry(),
129+
class_registry,
130130
&mut data_query_result,
131-
&mut self.view_states.lock(),
131+
self.view_states.lock().get_mut_or_create(*view_id, class),
132132
);
133133

134134
query_results.insert(*view_id, data_query_result);

crates/viewer/re_viewer/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ itertools.workspace = true
133133
jiff.workspace = true
134134
parking_lot.workspace = true
135135
poll-promise = { workspace = true, features = ["web"] }
136+
rayon.workspace = true
136137
rfd.workspace = true
137138
ron.workspace = true
138139
serde = { workspace = true, features = ["derive"] }

crates/viewer/re_viewer/src/app_state.rs

Lines changed: 105 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{borrow::Cow, str::FromStr as _};
33
use ahash::HashMap;
44
use egui::{Ui, text_edit::TextEditState, text_selection::LabelSelectionState};
55

6-
use re_chunk::TimelineName;
6+
use re_chunk::{Timeline, TimelineName};
77
use re_chunk_store::LatestAtQuery;
88
use re_entity_db::EntityDb;
99
use re_log_types::{AbsoluteTimeRangeF, DataSourceMessage, StoreId, TableId};
@@ -14,9 +14,10 @@ use re_types::blueprint::components::PanelState;
1414
use re_ui::{ContextExt as _, UiExt as _};
1515
use re_viewer_context::{
1616
AppOptions, ApplicationSelectionState, AsyncRuntimeHandle, BlueprintUndoState, CommandSender,
17-
ComponentUiRegistry, DisplayMode, DragAndDropManager, GlobalContext, Item, PlayState,
18-
RecordingConfig, SelectionChange, StorageContext, StoreContext, StoreHub, SystemCommand,
19-
SystemCommandSender as _, TableStore, ViewClassRegistry, ViewStates, ViewerContext,
17+
ComponentUiRegistry, DataQueryResult, DisplayMode, DragAndDropManager, GlobalContext,
18+
IndicatedEntities, Item, MaybeVisualizableEntities, PerVisualizer, PlayState, RecordingConfig,
19+
SelectionChange, StorageContext, StoreContext, StoreHub, SystemCommand,
20+
SystemCommandSender as _, TableStore, ViewClassRegistry, ViewId, ViewStates, ViewerContext,
2021
blueprint_timeline,
2122
open_url::{self, ViewerOpenUrl},
2223
};
@@ -278,7 +279,7 @@ impl AppState {
278279
view_class_registry.indicated_entities_per_visualizer(recording.store_id());
279280

280281
// Execute the queries for every `View`
281-
let mut query_results = {
282+
let query_results = {
282283
re_tracing::profile_scope!("query_results");
283284
viewport_ui
284285
.blueprint
@@ -359,39 +360,18 @@ impl AppState {
359360
// Update the viewport. May spawn new views and handle queued requests (like screenshots).
360361
viewport_ui.on_frame_start(&ctx);
361362

362-
for view in viewport_ui.blueprint.views.values() {
363-
if let Some(query_result) = query_results.get_mut(&view.id) {
364-
// TODO(andreas): This needs to be done in a store subscriber that exists per view (instance, not class!).
365-
// Note that right now we determine *all* visualizable entities, not just the queried ones.
366-
// In a store subscriber set this is fine, but on a per-frame basis it's wasteful.
367-
let visualizable_entities = view
368-
.class(view_class_registry)
369-
.determine_visualizable_entities(
370-
&maybe_visualizable_entities_per_visualizer,
371-
recording,
372-
&view_class_registry
373-
.new_visualizer_collection(view.class_identifier()),
374-
&view.space_origin,
375-
);
376-
377-
let resolver = re_viewport_blueprint::DataQueryPropertyResolver::new(
378-
view,
379-
view_class_registry,
380-
&maybe_visualizable_entities_per_visualizer,
381-
&visualizable_entities,
382-
&indicated_entities_per_visualizer,
383-
);
384-
385-
resolver.update_overrides(
386-
store_context.blueprint,
387-
&blueprint_query,
388-
rec_cfg.time_ctrl.read().timeline(),
389-
view_class_registry,
390-
query_result,
391-
view_states,
392-
);
393-
}
394-
}
363+
let active_timeline = *rec_cfg.time_ctrl.read().timeline();
364+
let query_results = update_overrides(
365+
store_context,
366+
query_results,
367+
view_class_registry,
368+
&viewport_ui.blueprint,
369+
&blueprint_query,
370+
&active_timeline,
371+
&maybe_visualizable_entities_per_visualizer,
372+
&indicated_entities_per_visualizer,
373+
view_states,
374+
);
395375

396376
// We need to recreate the context to appease the borrow checker. It is a bit annoying, but
397377
// it's just a bunch of refs so not really that big of a deal in practice.
@@ -760,6 +740,93 @@ impl AppState {
760740
}
761741
}
762742

743+
/// Updates the query results for the given viewport UI.
744+
///
745+
/// Returns query results derived from the previous one.
746+
#[expect(clippy::too_many_arguments)]
747+
fn update_overrides(
748+
store_context: &StoreContext<'_>,
749+
mut query_results: HashMap<ViewId, DataQueryResult>,
750+
view_class_registry: &ViewClassRegistry,
751+
viewport_blueprint: &ViewportBlueprint,
752+
blueprint_query: &LatestAtQuery,
753+
active_timeline: &Timeline,
754+
maybe_visualizable_entities_per_visualizer: &PerVisualizer<MaybeVisualizableEntities>,
755+
indicated_entities_per_visualizer: &PerVisualizer<IndicatedEntities>,
756+
view_states: &mut ViewStates,
757+
) -> HashMap<ViewId, DataQueryResult> {
758+
use rayon::iter::{IntoParallelIterator as _, ParallelIterator as _};
759+
760+
struct OverridesUpdateTask<'a> {
761+
view: &'a re_viewport_blueprint::ViewBlueprint,
762+
view_state: &'a dyn re_viewer_context::ViewState,
763+
query_result: DataQueryResult,
764+
}
765+
766+
for view in viewport_blueprint.views.values() {
767+
view_states.ensure_state_exists(view.id, view.class(view_class_registry));
768+
}
769+
770+
let work_items = viewport_blueprint
771+
.views
772+
.values()
773+
.filter_map(|view| {
774+
query_results.remove(&view.id).map(|query_result| {
775+
let view_state = view_states
776+
.get(view.id)
777+
.expect("View state should exist, we just called ensure_state_exists on it.");
778+
OverridesUpdateTask {
779+
view,
780+
view_state,
781+
query_result,
782+
}
783+
})
784+
})
785+
.collect::<Vec<_>>();
786+
787+
work_items
788+
.into_par_iter()
789+
.map(
790+
|OverridesUpdateTask {
791+
view,
792+
view_state,
793+
mut query_result,
794+
}| {
795+
// TODO(andreas): This needs to be done in a store subscriber that exists per view (instance, not class!).
796+
// Note that right now we determine *all* visualizable entities, not just the queried ones.
797+
// In a store subscriber set this is fine, but on a per-frame basis it's wasteful.
798+
let visualizable_entities = view
799+
.class(view_class_registry)
800+
.determine_visualizable_entities(
801+
maybe_visualizable_entities_per_visualizer,
802+
store_context.recording,
803+
&view_class_registry.new_visualizer_collection(view.class_identifier()),
804+
&view.space_origin,
805+
);
806+
807+
let resolver = re_viewport_blueprint::DataQueryPropertyResolver::new(
808+
view,
809+
view_class_registry,
810+
maybe_visualizable_entities_per_visualizer,
811+
&visualizable_entities,
812+
indicated_entities_per_visualizer,
813+
);
814+
815+
resolver.update_overrides(
816+
store_context.blueprint,
817+
blueprint_query,
818+
active_timeline,
819+
view_class_registry,
820+
&mut query_result,
821+
view_state,
822+
);
823+
824+
(view.id, query_result)
825+
},
826+
)
827+
.collect()
828+
}
829+
763830
fn table_ui(
764831
ctx: &ViewerContext<'_>,
765832
runtime: &AsyncRuntimeHandle,

crates/viewer/re_viewer_context/src/lib.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,12 @@ pub use self::{
8484
DataBasedVisualizabilityFilter, DataResult, IdentifiedViewSystem,
8585
OptionalViewEntityHighlight, OverridePath, PerSystemDataResults, PerSystemEntities,
8686
PropertyOverrides, RecommendedView, SmallVisualizerSet, SystemExecutionOutput, ViewClass,
87-
ViewClassExt, ViewClassLayoutPriority, ViewClassRegistry, ViewClassRegistryError,
88-
ViewContext, ViewContextCollection, ViewContextSystem, ViewEntityHighlight, ViewHighlights,
89-
ViewOutlineMasks, ViewQuery, ViewSpawnHeuristics, ViewState, ViewStateExt, ViewStates,
90-
ViewSystemExecutionError, ViewSystemIdentifier, ViewSystemRegistrator,
91-
VisualizableFilterContext, VisualizerCollection, VisualizerQueryInfo, VisualizerSystem,
87+
ViewClassExt, ViewClassLayoutPriority, ViewClassPlaceholder, ViewClassRegistry,
88+
ViewClassRegistryError, ViewContext, ViewContextCollection, ViewContextSystem,
89+
ViewEntityHighlight, ViewHighlights, ViewOutlineMasks, ViewQuery, ViewSpawnHeuristics,
90+
ViewState, ViewStateExt, ViewStates, ViewSystemExecutionError, ViewSystemIdentifier,
91+
ViewSystemRegistrator, VisualizableFilterContext, VisualizerCollection,
92+
VisualizerQueryInfo, VisualizerSystem,
9293
},
9394
viewer_context::{RecordingConfig, ViewerContext},
9495
visitor_flow_control::VisitorControlFlow,

crates/viewer/re_viewer_context/src/view/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub use view_class::{
2828
ViewClass, ViewClassExt, ViewClassLayoutPriority, ViewState, ViewStateExt,
2929
VisualizableFilterContext,
3030
};
31+
pub use view_class_placeholder::ViewClassPlaceholder;
3132
pub use view_class_registry::{ViewClassRegistry, ViewClassRegistryError, ViewSystemRegistrator};
3233
pub use view_context::ViewContext;
3334
pub use view_context_system::{ViewContextCollection, ViewContextSystem};

crates/viewer/re_viewer_context/src/view/view_class_registry.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ impl ViewClassRegistry {
297297
)
298298
}
299299

300-
/// For each visualizer, the set of entities that have at least one matching indicator component.
300+
/// For each visualizer, the set of entities that have at least one component with a matching archetype name.
301301
pub fn indicated_entities_per_visualizer(
302302
&self,
303303
store_id: &re_log_types::StoreId,

crates/viewer/re_viewer_context/src/view/visualizer_entity_subscriber.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,12 @@ impl VisualizerEntitySubscriber {
124124
.map(|mapping| &mapping.maybe_visualizable_entities)
125125
}
126126

127-
/// List of entities that at some point in time had any of the indicator components advertised by this visualizer.
127+
/// List of entities that at some point in time had a component of an archetypes matching the visualizer's query.
128128
///
129129
/// Useful for quickly evaluating basic "should this visualizer apply by default"-heuristic.
130130
/// Does *not* imply that any of the given entities is also in the (maybe-)visualizable-set!
131131
///
132-
/// If the visualizer has no indicator components, this list will contain all entities in the store.
132+
/// If the visualizer has no archetypes, this list will contain all entities in the store.
133133
pub fn indicated_entities(&self, store: &StoreId) -> Option<&IndicatedEntities> {
134134
self.per_store_mapping
135135
.get(store)

crates/viewer/re_viewport_blueprint/src/view.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ mod tests {
463463
use re_types::{ComponentDescriptor, blueprint::archetypes::EntityBehavior};
464464
use re_viewer_context::{
465465
IndicatedEntities, MaybeVisualizableEntities, OverridePath, PerVisualizer,
466-
VisualizableEntities,
466+
ViewClassPlaceholder, VisualizableEntities,
467467
};
468468

469469
use crate::view_contents::DataQueryPropertyResolver;
@@ -516,7 +516,8 @@ mod tests {
516516
);
517517

518518
// Basic blueprint - a single view that queries everything.
519-
let view = ViewBlueprint::new_with_root_wildcard("3D".into());
519+
test_ctx.register_view_class::<ViewClassPlaceholder>();
520+
let view = ViewBlueprint::new_with_root_wildcard(ViewClassPlaceholder::identifier());
520521
let override_root = ViewContents::override_path_for_entity(view.id, &EntityPath::root());
521522

522523
// Things needed to resolve properties:
@@ -764,14 +765,20 @@ mod tests {
764765
visualizable_entities,
765766
);
766767
let mut view_states = ViewStates::default();
768+
let view_state = view_states.get_mut_or_create(
769+
view.id,
770+
ctx.view_class_registry
771+
.class(view.class_identifier())
772+
.expect("view class should be registered"),
773+
);
767774

768775
resolver.update_overrides(
769776
ctx.blueprint_db(),
770777
ctx.blueprint_query,
771778
ctx.rec_cfg.time_ctrl.read().timeline(),
772779
ctx.view_class_registry(),
773780
&mut query_result,
774-
&mut view_states,
781+
view_state,
775782
);
776783

777784
result = Some(query_result.clone());

crates/viewer/re_viewport_blueprint/src/view_contents.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use re_types::{
2222
use re_viewer_context::{
2323
DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree,
2424
IndicatedEntities, MaybeVisualizableEntities, OverridePath, PerVisualizer, PropertyOverrides,
25-
QueryRange, ViewClassRegistry, ViewId, ViewStates, ViewerContext, VisualizableEntities,
25+
QueryRange, ViewClassRegistry, ViewId, ViewState, ViewerContext, VisualizableEntities,
2626
};
2727

2828
use crate::{ViewBlueprint, ViewProperty};
@@ -462,6 +462,8 @@ impl<'a> DataQueryPropertyResolver<'a> {
462462
parent_visible: bool,
463463
parent_interactive: bool,
464464
) {
465+
// This is called very frequently, don't put a profile scope here.
466+
465467
let Some(node) = query_result.tree.lookup_node_mut(handle) else {
466468
return;
467469
};
@@ -594,14 +596,11 @@ impl<'a> DataQueryPropertyResolver<'a> {
594596
active_timeline: &Timeline,
595597
view_class_registry: &ViewClassRegistry,
596598
query_result: &mut DataQueryResult,
597-
view_states: &mut ViewStates,
599+
view_state: &dyn ViewState,
598600
) {
599-
// This is called very frequently, don't put a profile scope here.
601+
re_tracing::profile_function!();
600602

601603
if let Some(root) = query_result.tree.root_handle() {
602-
let class = self.view.class(view_class_registry);
603-
let view_state = view_states.get_mut_or_create(self.view.id, class);
604-
605604
let default_query_range = self.view.query_range(
606605
blueprint,
607606
blueprint_query,

0 commit comments

Comments
 (0)