Skip to content

Commit

Permalink
Improved automatic view creation heuristic, major speedup for scenes …
Browse files Browse the repository at this point in the history
…with many entities (#4874)

### What

What space views get spawned is no longer determined by a centralized
heuristic, but instead by one distributed to each space view class. Each
of those heuristics can make much better use of existing information
provided by store subscribers.

* Fixes most of #4388
   *  a few things on the tracking issue still left to investigate
* Fixes #3342 
* more "obsoleted" arguably: We originally had a hack that 1d tensor
logging would display a bar chart. That's not actually desirable. There
is still the open issue for the _query_ now to not rely exclusively on
indicators, but automatic view spawning is encouraged to do so. (Overall
the workings and fault lines of these systems have shifted a lot since
the ticket was filed.)
* Fixes #4695
* Looks like this now:
![image](https://github.com/rerun-io/rerun/assets/1220815/a77e7768-0d4f-4045-b5a2-980e49955c31)
* Fixes #4689
* Fixes #3079

Significant speedup for scenes with many entities (100+):

`opf --no-frames`
before: `App::update` 39.0ms, of which `default_created_space_views`
10.4ms
after: `App::update` 27.4ms, of which `default_created_space_views`
0.17ms

`python/tests/many_entities (1000 individual points)`
before: `App:::update` 151ms, of which `default_created_space_views`
124ms
after: `App::update` 22.6ms, of which `default_created_space_views`
0.06ms
(_still pretty bad, but less so and for different reasons now!_)

(numbers from my macbook on a release build. Actual numbers might differ
by now, these are from last week. )


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4874/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4874/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4874/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4874)
- [Docs
preview](https://rerun.io/preview/ce72c01b3379c20b43d3cdbbe44c37dce2a94f5c/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/ce72c01b3379c20b43d3cdbbe44c37dce2a94f5c/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf authored Jan 28, 2024
1 parent 7accc2f commit 15cd60d
Show file tree
Hide file tree
Showing 40 changed files with 657 additions and 592 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/re_data_store/src/store_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl DataStore {
} else {
// Cache miss! Craft a new instance keys from the ground up.

// ...so we create it manually instead.
// so we create it manually instead.
let values =
arrow2::array::UInt64Array::from_vec((0..num_instances as u64).collect_vec())
.boxed();
Expand Down
6 changes: 3 additions & 3 deletions crates/re_entity_db/src/entity_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,9 @@ impl EntityTree {
subtree_recursive(self, path.as_slice())
}

// Invokes visitor for `self` all children recursively.
pub fn visit_children_recursively(&self, visitor: &mut impl FnMut(&EntityPath)) {
visitor(&self.path);
// Invokes visitor for `self` and all children recursively.
pub fn visit_children_recursively(&self, visitor: &mut impl FnMut(&EntityPath, &EntityInfo)) {
visitor(&self.path, &self.entity);
for child in self.children.values() {
child.visit_children_recursively(visitor);
}
Expand Down
14 changes: 14 additions & 0 deletions crates/re_log_types/src/path/entity_path_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,20 @@ impl EntityPathFilter {
filter
}

/// Creates a new entity path filter that includes only a single entity.
pub fn single_entity_filter(entity_path: &EntityPath) -> Self {
let mut filter = Self::default();
filter.add_exact(entity_path.clone());
filter
}

/// Creates a new entity path filter that includes a single subtree.
pub fn subtree_entity_filter(entity_path: &EntityPath) -> Self {
let mut filter = Self::default();
filter.add_subtree(entity_path.clone());
filter
}

pub fn add_rule(&mut self, effect: RuleEffect, rule: EntityPathRule) {
self.rules.insert(rule, effect);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ impl RecordingStream {
// Get the current time on all timelines, for the current recording, on the current
// thread…
let mut now = self.now();
// ...and then also inject the current recording tick into it.
// and then also inject the current recording tick into it.
now.insert(Timeline::log_tick(), tick.into());

// Inject all these times into the row, overriding conflicting times, if any.
Expand Down
2 changes: 1 addition & 1 deletion crates/re_space_view/src/data_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ pub trait DataQuery {
&self,
ctx: &StoreContext<'_>,
visualizable_entities_for_visualizer_systems: &PerVisualizer<VisualizableEntities>,
indicator_matching_entities_per_visualizer: &PerVisualizer<IndicatorMatchingEntities>,
indicated_entities_per_visualizer: &PerVisualizer<IndicatorMatchingEntities>,
) -> DataQueryResult;
}
46 changes: 20 additions & 26 deletions crates/re_space_view/src/data_query_blueprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl DataQuery for DataQueryBlueprint {
&self,
ctx: &re_viewer_context::StoreContext<'_>,
visualizable_entities_for_visualizer_systems: &PerVisualizer<VisualizableEntities>,
indicator_matching_entities_per_visualizer: &PerVisualizer<IndicatorMatchingEntities>,
indicated_entities_per_visualizer: &PerVisualizer<IndicatorMatchingEntities>,
) -> DataQueryResult {
re_tracing::profile_function!();

Expand All @@ -190,7 +190,7 @@ impl DataQuery for DataQueryBlueprint {
let executor = QueryExpressionEvaluator::new(
self,
visualizable_entities_for_visualizer_systems,
indicator_matching_entities_per_visualizer,
indicated_entities_per_visualizer,
);

let root_handle = ctx.recording.and_then(|store| {
Expand All @@ -212,16 +212,15 @@ impl DataQuery for DataQueryBlueprint {
/// to a pure recursive evaluation.
struct QueryExpressionEvaluator<'a> {
visualizable_entities_for_visualizer_systems: &'a PerVisualizer<VisualizableEntities>,
indicator_matching_entities_per_visualizer:
&'a IntMap<ViewSystemIdentifier, IndicatorMatchingEntities>,
indicated_entities_per_visualizer: &'a IntMap<ViewSystemIdentifier, IndicatorMatchingEntities>,
entity_path_filter: EntityPathFilter,
}

impl<'a> QueryExpressionEvaluator<'a> {
fn new(
blueprint: &'a DataQueryBlueprint,
visualizable_entities_for_visualizer_systems: &'a PerVisualizer<VisualizableEntities>,
indicator_matching_entities_per_visualizer: &'a IntMap<
indicated_entities_per_visualizer: &'a IntMap<
ViewSystemIdentifier,
IndicatorMatchingEntities,
>,
Expand All @@ -230,7 +229,7 @@ impl<'a> QueryExpressionEvaluator<'a> {

Self {
visualizable_entities_for_visualizer_systems,
indicator_matching_entities_per_visualizer,
indicated_entities_per_visualizer,
entity_path_filter: blueprint.entity_path_filter.clone(),
}
}
Expand Down Expand Up @@ -272,11 +271,9 @@ impl<'a> QueryExpressionEvaluator<'a> {
// to a single `IntSet<EntityPathHash>`
// -> consider three steps of query: list entities, list their visualizers, list their properties
if self
.indicator_matching_entities_per_visualizer
.indicated_entities_per_visualizer
.get(visualizer)
.map_or(false, |matching_list| {
matching_list.contains(&entity_path.hash())
})
.map_or(false, |matching_list| matching_list.contains(entity_path))
{
Some(*visualizer)
} else {
Expand Down Expand Up @@ -404,7 +401,7 @@ impl DataQueryPropertyResolver<'_> {
let mut prop_map = self.auto_properties.clone();

if let Some(tree) = blueprint.tree().subtree(override_root) {
tree.visit_children_recursively(&mut |path: &EntityPath| {
tree.visit_children_recursively(&mut |path: &EntityPath, _| {
if let Some(props) = blueprint
.store()
.query_latest_component_quiet::<EntityPropertiesComponent>(path, query)
Expand Down Expand Up @@ -660,24 +657,21 @@ mod tests {
entity_path_filter: EntityPathFilter::parse_forgiving(filter),
};

let indicator_matching_entities_per_visualizer =
PerVisualizer::<IndicatorMatchingEntities>(
visualizable_entities_for_visualizer_systems
.iter()
.map(|(id, entities)| {
(
*id,
IndicatorMatchingEntities(
entities.0.iter().map(|path| path.hash()).collect(),
),
)
})
.collect(),
);
let indicated_entities_per_visualizer = PerVisualizer::<IndicatorMatchingEntities>(
visualizable_entities_for_visualizer_systems
.iter()
.map(|(id, entities)| {
(
*id,
IndicatorMatchingEntities(entities.0.iter().cloned().collect()),
)
})
.collect(),
);
let query_result = query.execute_query(
&ctx,
&visualizable_entities_for_visualizer_systems,
&indicator_matching_entities_per_visualizer,
&indicated_entities_per_visualizer,
);

let mut visited = vec![];
Expand Down
58 changes: 58 additions & 0 deletions crates/re_space_view/src/heuristics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use re_log_types::EntityPathFilter;
use re_viewer_context::{
ApplicableEntities, IdentifiedViewSystem, RecommendedSpaceView, SpaceViewClass,
SpaceViewSpawnHeuristics, ViewerContext, VisualizerSystem,
};

/// Spawns a space view for each single entity which is visualizable & indicator-matching for a given visualizer.
///
/// This is used as utility by *some* space view types that want
/// to spawn a space view for every single entity that is visualizable with a given visualizer.
pub fn suggest_space_view_for_each_entity<TVisualizer>(
ctx: &ViewerContext<'_>,
space_view: &impl SpaceViewClass,
) -> SpaceViewSpawnHeuristics
where
TVisualizer: VisualizerSystem + IdentifiedViewSystem + Default,
{
re_tracing::profile_function!();

let Some(indicator_matching_entities) = ctx
.indicated_entities_per_visualizer
.get(&TVisualizer::identifier())
else {
return Default::default();
};
let Some(applicable_entities) = ctx
.applicable_entities_per_visualizer
.get(&TVisualizer::identifier())
else {
return Default::default();
};

let visualizer = TVisualizer::default();
let recommended_space_views = applicable_entities
.intersection(indicator_matching_entities)
.filter_map(|entity| {
let context = space_view.visualizable_filter_context(entity, ctx.entity_db);
if visualizer
.filter_visualizable_entities(
ApplicableEntities(std::iter::once(entity.clone()).collect()),
context.as_ref(),
)
.is_empty()
{
None
} else {
Some(RecommendedSpaceView {
root: entity.clone(),
query_filter: EntityPathFilter::single_entity_filter(entity),
})
}
})
.collect();

re_viewer_context::SpaceViewSpawnHeuristics {
recommended_space_views,
}
}
2 changes: 2 additions & 0 deletions crates/re_space_view/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ pub mod blueprint;
pub mod controls;
mod data_query;
mod data_query_blueprint;
mod heuristics;
mod screenshot;
mod unreachable_transform_reason;

pub use data_query::{DataQuery, EntityOverrideContext, PropertyResolver};
pub use data_query_blueprint::DataQueryBlueprint;
pub use heuristics::suggest_space_view_for_each_entity;
pub use screenshot::ScreenshotMode;
pub use unreachable_transform_reason::UnreachableTransformReason;

Expand Down
10 changes: 9 additions & 1 deletion crates/re_space_view_bar_chart/src/space_view_class.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use egui::util::hash;
use re_entity_db::{EditableAutoValue, EntityProperties, LegendCorner};
use re_log_types::EntityPath;
use re_space_view::controls;
use re_space_view::{controls, suggest_space_view_for_each_entity};
use re_types::datatypes::TensorBuffer;
use re_viewer_context::{
auto_color, SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId,
Expand Down Expand Up @@ -59,6 +59,14 @@ impl SpaceViewClass for BarChartSpaceView {
None
}

fn spawn_heuristics(
&self,
ctx: &ViewerContext<'_>,
) -> re_viewer_context::SpaceViewSpawnHeuristics {
re_tracing::profile_function!();
suggest_space_view_for_each_entity::<BarChartVisualizerSystem>(ctx, self)
}

fn layout_priority(&self) -> re_viewer_context::SpaceViewClassLayoutPriority {
re_viewer_context::SpaceViewClassLayoutPriority::Low
}
Expand Down
12 changes: 2 additions & 10 deletions crates/re_space_view_bar_chart/src/visualizer_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ use re_data_store::LatestAtQuery;
use re_entity_db::EntityPath;
use re_space_view::diff_component_filter;
use re_types::{
archetypes::{BarChart, Tensor},
components::Color,
datatypes::TensorData,
Archetype, ComponentNameSet,
archetypes::BarChart, components::Color, datatypes::TensorData, Archetype, ComponentNameSet,
};
use re_viewer_context::{
IdentifiedViewSystem, SpaceViewSystemExecutionError, ViewContextCollection, ViewQuery,
Expand Down Expand Up @@ -45,12 +42,7 @@ impl VisualizerSystem for BarChartVisualizerSystem {
}

fn indicator_components(&self) -> ComponentNameSet {
// TODO(#3342): For now, we relax the indicator component heuristics on bar charts so that
// logging a 1D tensor also results in a bar chart view, rather than a broken viewer (see #3709).
// Ideally though, this should be implemented using an heuristic fallback mechanism.
[BarChart::indicator().name(), Tensor::indicator().name()]
.into_iter()
.collect()
std::iter::once(BarChart::indicator().name()).collect()
}

fn applicability_filter(&self) -> Option<Box<dyn VisualizerAdditionalApplicabilityFilter>> {
Expand Down
14 changes: 6 additions & 8 deletions crates/re_space_view_dataframe/src/space_view_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ use re_entity_db::{EntityProperties, InstancePath};
use re_log_types::{EntityPath, Timeline};
use re_query::get_component_with_instances;
use re_viewer_context::{
AutoSpawnHeuristic, PerSystemEntities, SpaceViewClass, SpaceViewClassRegistryError,
SpaceViewId, SpaceViewSystemExecutionError, SystemExecutionOutput, UiVerbosity, ViewQuery,
ViewerContext,
SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId, SpaceViewSystemExecutionError,
SystemExecutionOutput, UiVerbosity, ViewQuery, ViewerContext,
};

use crate::visualizer_system::EmptySystem;
Expand Down Expand Up @@ -53,13 +52,12 @@ impl SpaceViewClass for DataframeSpaceView {
re_viewer_context::SpaceViewClassLayoutPriority::Low
}

fn auto_spawn_heuristic(
fn spawn_heuristics(
&self,
_ctx: &ViewerContext<'_>,
_space_origin: &EntityPath,
_ent_paths: &PerSystemEntities,
) -> re_viewer_context::AutoSpawnHeuristic {
AutoSpawnHeuristic::NeverSpawn
) -> re_viewer_context::SpaceViewSpawnHeuristics {
// Doesn't spawn anything by default.
Default::default()
}

fn selection_ui(
Expand Down
Loading

0 comments on commit 15cd60d

Please sign in to comment.