Skip to content

Commit

Permalink
Store indicator component mapping as part of Visualizer's store subsc…
Browse files Browse the repository at this point in the history
…ription (#4608)

### What
 
* Kinda part of #4388
* Continuing the spirit of #4604
    * Exact same reasoning and motiviation applies! 
* This time this actually has clear positive perf implications:
Evaluation of `identify_entities_per_system_per_class` goes from 1ms to
0.5ms on `opf --no-frames`
* but again in the "heuristic evaluation is part of query"-world this is
a _huge_ speedup


### 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/4608/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4608/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/4608/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/4608)
- [Docs
preview](https://rerun.io/preview/43ecf8119748c2370d859cade05837fd62f9ffe9/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/43ecf8119748c2370d859cade05837fd62f9ffe9/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 Dec 22, 2023
1 parent c621d90 commit e70e1dd
Show file tree
Hide file tree
Showing 13 changed files with 149 additions and 128 deletions.
16 changes: 7 additions & 9 deletions crates/re_space_view_spatial/src/parts/boxes2d.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use re_arrow_store::LatestAtQuery;
use nohash_hasher::IntSet;
use re_data_store::{EntityPath, InstancePathHash};
use re_log_types::EntityPathHash;
use re_query::{ArchetypeView, QueryError};
use re_types::{
archetypes::Boxes2D,
components::{HalfSizes2D, Position2D, Text},
Archetype, ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, HeuristicFilterContext, IdentifiedViewSystem,
ResolvedAnnotationInfos, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem,
ViewQuery, ViewerContext,
HeuristicFilterContext, IdentifiedViewSystem, ResolvedAnnotationInfos,
SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext,
};

use crate::{
Expand Down Expand Up @@ -181,13 +181,11 @@ impl ViewPartSystem for Boxes2DPart {

fn heuristic_filter(
&self,
_store: &re_arrow_store::DataStore,
_ent_path: &EntityPath,
entities_with_matching_indicator: &IntSet<EntityPathHash>,
ent_path: &EntityPath,
ctx: HeuristicFilterContext,
_query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
if !entities_with_matching_indicator.contains(&ent_path.hash()) {
return false;
}

Expand Down
11 changes: 4 additions & 7 deletions crates/re_space_view_spatial/src/parts/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use egui::NumExt;
use itertools::Itertools as _;
use nohash_hasher::IntSet;

use re_arrow_store::LatestAtQuery;
use re_data_store::{EntityPath, EntityProperties};
use re_log_types::{EntityPathHash, RowId};
use re_query::{ArchetypeView, QueryError};
Expand All @@ -20,7 +19,7 @@ use re_types::{
Archetype as _, ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, gpu_bridge, DefaultColor, HeuristicFilterContext, SpaceViewClass,
gpu_bridge, DefaultColor, HeuristicFilterContext, SpaceViewClass,
SpaceViewSystemExecutionError, TensorDecodeCache, TensorStatsCache, ViewPartSystem, ViewQuery,
ViewerContext, VisualizerAdditionalApplicabilityFilter,
};
Expand Down Expand Up @@ -694,13 +693,11 @@ impl ViewPartSystem for ImagesPart {

fn heuristic_filter(
&self,
_store: &re_arrow_store::DataStore,
_ent_path: &EntityPath,
entities_with_matching_indicator: &IntSet<EntityPathHash>,
ent_path: &EntityPath,
ctx: HeuristicFilterContext,
_query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
if !entities_with_matching_indicator.contains(&ent_path.hash()) {
return false;
}

Expand Down
16 changes: 7 additions & 9 deletions crates/re_space_view_spatial/src/parts/lines2d.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use re_arrow_store::LatestAtQuery;
use nohash_hasher::IntSet;
use re_data_store::{EntityPath, InstancePathHash};
use re_log_types::EntityPathHash;
use re_query::{ArchetypeView, QueryError};
use re_types::{
archetypes::LineStrips2D,
components::{LineStrip2D, Text},
Archetype as _, ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, HeuristicFilterContext, IdentifiedViewSystem,
ResolvedAnnotationInfos, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem,
ViewQuery, ViewerContext,
HeuristicFilterContext, IdentifiedViewSystem, ResolvedAnnotationInfos,
SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext,
};

use crate::{
Expand Down Expand Up @@ -176,13 +176,11 @@ impl ViewPartSystem for Lines2DPart {

fn heuristic_filter(
&self,
_store: &re_arrow_store::DataStore,
_ent_path: &EntityPath,
entities_with_matching_indicator: &IntSet<EntityPathHash>,
ent_path: &EntityPath,
ctx: HeuristicFilterContext,
_query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
if !entities_with_matching_indicator.contains(&ent_path.hash()) {
return false;
}

Expand Down
16 changes: 7 additions & 9 deletions crates/re_space_view_spatial/src/parts/points2d.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use re_arrow_store::LatestAtQuery;
use nohash_hasher::IntSet;
use re_data_store::{EntityPath, InstancePathHash};
use re_log_types::EntityPathHash;
use re_query::{ArchetypeView, QueryError};
use re_types::{
archetypes::Points2D,
components::{Position2D, Text},
Archetype, ComponentNameSet,
};
use re_viewer_context::{
default_heuristic_filter, HeuristicFilterContext, IdentifiedViewSystem,
ResolvedAnnotationInfos, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem,
ViewQuery, ViewerContext,
HeuristicFilterContext, IdentifiedViewSystem, ResolvedAnnotationInfos,
SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext,
};

use crate::{
Expand Down Expand Up @@ -203,13 +203,11 @@ impl ViewPartSystem for Points2DPart {

fn heuristic_filter(
&self,
_store: &re_arrow_store::DataStore,
_ent_path: &EntityPath,
entities_with_matching_indicator: &IntSet<EntityPathHash>,
ent_path: &EntityPath,
ctx: HeuristicFilterContext,
_query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
if !default_heuristic_filter(entity_components, &self.indicator_components()) {
if !entities_with_matching_indicator.contains(&ent_path.hash()) {
return false;
}

Expand Down
10 changes: 9 additions & 1 deletion crates/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,14 @@ impl AppState {
let rec_cfg =
recording_config_entry(recording_configs, store_db.store_id().clone(), store_db);

// Gather all entities that have a matching indicator for a visualizer.
let entities_with_matching_indicator_per_visualizer = space_view_class_registry
.entities_with_matching_indicator_per_visualizer(store_db.store_id());

let entities_per_system_per_class = identify_entities_per_system_per_class(
&entities_with_matching_indicator_per_visualizer,
space_view_class_registry,
store_db,
&rec_cfg.time_ctrl.get_mut().current_query(),
);

// Execute the queries for every `SpaceView`
Expand Down Expand Up @@ -169,6 +173,8 @@ impl AppState {
store_db,
store_context,
entities_per_system_per_class: &entities_per_system_per_class,
entities_with_matching_indicator_per_visualizer:
&entities_with_matching_indicator_per_visualizer,
query_results: &query_results,
rec_cfg,
re_ui,
Expand Down Expand Up @@ -206,6 +212,8 @@ impl AppState {
store_db,
store_context,
entities_per_system_per_class: &entities_per_system_per_class,
entities_with_matching_indicator_per_visualizer:
&entities_with_matching_indicator_per_visualizer,
query_results: &query_results,
rec_cfg,
re_ui,
Expand Down
10 changes: 5 additions & 5 deletions crates/re_viewer_context/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ pub use selection_state::{
Selection, SelectionHighlight,
};
pub use space_view::{
default_heuristic_filter, AutoSpawnHeuristic, DataResult, DynSpaceViewClass,
HeuristicFilterContext, IdentifiedViewSystem, PerSystemDataResults, PerSystemEntities,
PropertyOverrides, SpaceViewClass, SpaceViewClassIdentifier, SpaceViewClassLayoutPriority,
SpaceViewClassRegistry, SpaceViewClassRegistryError, SpaceViewEntityHighlight,
SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewState, SpaceViewSystemExecutionError,
AutoSpawnHeuristic, DataResult, DynSpaceViewClass, HeuristicFilterContext,
IdentifiedViewSystem, PerSystemDataResults, PerSystemEntities, PropertyOverrides,
SpaceViewClass, SpaceViewClassIdentifier, SpaceViewClassLayoutPriority, SpaceViewClassRegistry,
SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights,
SpaceViewOutlineMasks, SpaceViewState, SpaceViewSystemExecutionError,
SpaceViewSystemRegistrator, SystemExecutionOutput, ViewContextCollection, ViewContextSystem,
ViewPartCollection, ViewPartSystem, ViewQuery, ViewSystemIdentifier,
VisualizerAdditionalApplicabilityFilter,
Expand Down
4 changes: 1 addition & 3 deletions crates/re_viewer_context/src/space_view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ pub use space_view_class_registry::{
};
pub use system_execution_output::SystemExecutionOutput;
pub use view_context_system::{ViewContextCollection, ViewContextSystem};
pub use view_part_system::{
default_heuristic_filter, HeuristicFilterContext, ViewPartCollection, ViewPartSystem,
};
pub use view_part_system::{HeuristicFilterContext, ViewPartCollection, ViewPartSystem};
pub use view_query::{DataResult, PerSystemDataResults, PropertyOverrides, ViewQuery};
pub use visualizer_entity_subscriber::VisualizerAdditionalApplicabilityFilter;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ahash::{HashMap, HashSet};
use nohash_hasher::IntSet;
use nohash_hasher::{IntMap, IntSet};
use re_arrow_store::DataStore;
use re_log_types::EntityPath;
use re_log_types::{EntityPath, EntityPathHash};

use crate::{
DynSpaceViewClass, IdentifiedViewSystem, SpaceViewClassIdentifier, ViewContextCollection,
Expand Down Expand Up @@ -284,17 +284,42 @@ impl SpaceViewClassRegistry {
pub fn applicable_entities_for_visualizer_system(
&self,
visualizer: ViewSystemIdentifier,
store: &re_log_types::StoreId,
store_id: &re_log_types::StoreId,
) -> Option<IntSet<EntityPath>> {
self.visualizers.get(&visualizer).and_then(|entry| {
DataStore::with_subscriber::<VisualizerEntitySubscriber, _, _>(
entry.entity_subscriber_handle,
|subscriber| subscriber.entities(store).cloned(),
|subscriber| subscriber.applicable_entities(store_id).cloned(),
)
.flatten()
})
}

/// For each visualizer, the set of entities that have at least one matching indicator component.
pub fn entities_with_matching_indicator_per_visualizer(
&self,
store_id: &re_log_types::StoreId,
) -> IntMap<ViewSystemIdentifier, IntSet<EntityPathHash>> {
self.visualizers
.iter()
.map(|(id, entry)| {
(
*id,
DataStore::with_subscriber::<VisualizerEntitySubscriber, _, _>(
entry.entity_subscriber_handle,
|subscriber| {
subscriber
.entities_with_matching_indicator(store_id)
.cloned()
},
)
.flatten()
.unwrap_or_default(),
)
})
.collect()
}

pub fn new_context_collection(
&self,
space_view_class_identifier: SpaceViewClassIdentifier,
Expand Down
37 changes: 5 additions & 32 deletions crates/re_viewer_context/src/space_view/view_part_system.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ahash::HashMap;

use re_arrow_store::LatestAtQuery;
use re_log_types::EntityPath;
use nohash_hasher::IntSet;
use re_log_types::{EntityPath, EntityPathHash};
use re_types::ComponentNameSet;

use crate::{
Expand Down Expand Up @@ -62,24 +62,16 @@ pub trait ViewPartSystem: Send + Sync + 'static {
/// the minimal set of required components), this method applies an arbitrary filter to determine whether
/// or not the system should be instantiated by default.
///
/// The passed-in set of `entity_components` corresponds to all the different components that have ever
/// been logged on the entity path.
///
/// By default, this returns true if eiher [`Self::indicator_components`] is empty or
/// `entity_components` contains at least one of these indicator components.
///
/// Override this method only if a more detailed condition is required to inform heuristics whether or not
/// the given entity is relevant for this system.
#[inline]
fn heuristic_filter(
&self,
_store: &re_arrow_store::DataStore,
_ent_path: &EntityPath,
entities_with_matching_indicator: &IntSet<EntityPathHash>,
ent_path: &EntityPath,
_ctx: HeuristicFilterContext,
_query: &LatestAtQuery,
entity_components: &ComponentNameSet,
) -> bool {
default_heuristic_filter(entity_components, &self.indicator_components())
entities_with_matching_indicator.contains(&ent_path.hash())
}

/// Additional filter for applicability.
Expand Down Expand Up @@ -115,25 +107,6 @@ pub trait ViewPartSystem: Send + Sync + 'static {
fn as_any(&self) -> &dyn std::any::Any;
}

/// The default implementation for [`ViewPartSystem::heuristic_filter`].
///
/// Returns true if either `indicator_components` is empty or `entity_components` contains at least one
/// of these indicator components.
///
/// Exported as a standalone function to simplify the implementation of custom filters.
#[inline]
pub fn default_heuristic_filter(
entity_components: &ComponentNameSet,
indicator_components: &ComponentNameSet,
) -> bool {
if indicator_components.is_empty() {
true // if there are no indicator components, then show anything with the required compoonents
} else {
// do we have at least one of the indicator components?
entity_components.intersection(indicator_components).count() > 0
}
}

pub struct ViewPartCollection {
pub systems: HashMap<ViewSystemIdentifier, Box<dyn ViewPartSystem>>,
}
Expand Down
Loading

0 comments on commit e70e1dd

Please sign in to comment.