Skip to content

Commit

Permalink
Don't run queries we know will be empty
Browse files Browse the repository at this point in the history
  • Loading branch information
@jleibs
jleibs committed Dec 15, 2023
1 parent 1493e68 commit 3f0cd4a
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 76 deletions.
4 changes: 2 additions & 2 deletions crates/re_space_view/src/data_query.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use re_data_store::{EntityProperties, EntityPropertyMap};
use re_viewer_context::{DataQueryResult, EntitiesPerSystemPerClass, StoreContext};
use re_viewer_context::{DataQueryResult, EntitiesPerSystem, StoreContext};

pub struct EntityOverrides {
pub root: EntityProperties,
Expand Down Expand Up @@ -43,6 +43,6 @@ pub trait DataQuery {
&self,
property_resolver: &impl PropertyResolver,
ctx: &StoreContext<'_>,
entities_per_system_per_class: &EntitiesPerSystemPerClass,
entities_per_system: &EntitiesPerSystem,
) -> DataQueryResult;
}
24 changes: 8 additions & 16 deletions crates/re_space_view/src/data_query_blueprint.rs
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use nohash_hasher::IntSet;
use once_cell::sync::Lazy;
use re_data_store::{
EntityProperties, EntityPropertiesComponent, EntityPropertyMap, EntityTree, StoreDb,
};
use re_log_types::{DataRow, EntityPath, EntityPathExpr, RowId, TimePoint};
use re_types_core::archetypes::Clear;
use re_viewer_context::{
DataQueryId, DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree,
EntitiesPerSystem, EntitiesPerSystemPerClass, SpaceViewClassIdentifier, SpaceViewId,
StoreContext, SystemCommand, SystemCommandSender as _, ViewerContext,
EntitiesPerSystem, SpaceViewClassIdentifier, SpaceViewId, StoreContext, SystemCommand,
SystemCommandSender as _, ViewerContext,
};
use slotmap::SlotMap;
use smallvec::SmallVec;
Expand Down Expand Up @@ -280,21 +279,15 @@ impl DataQuery for DataQueryBlueprint {
&self,
property_resolver: &impl PropertyResolver,
ctx: &re_viewer_context::StoreContext<'_>,
entities_per_system_per_class: &EntitiesPerSystemPerClass,
entities_per_system: &EntitiesPerSystem,
) -> DataQueryResult {
re_tracing::profile_function!();

static EMPTY_ENTITY_LIST: Lazy<EntitiesPerSystem> = Lazy::new(Default::default);

let mut data_results = SlotMap::<DataResultHandle, DataResultNode>::default();

let overrides = property_resolver.resolve_entity_overrides(ctx);

let per_system_entity_list = entities_per_system_per_class
.get(&self.space_view_class_identifier)
.unwrap_or(&EMPTY_ENTITY_LIST);

let executor = QueryExpressionEvaluator::new(self, per_system_entity_list);
let executor = QueryExpressionEvaluator::new(self, entities_per_system);

let root_handle = ctx.recording.and_then(|store| {
executor.add_entity_tree_to_data_results_recursive(
Expand Down Expand Up @@ -633,10 +626,9 @@ mod tests {
recording.add_data_row(row).unwrap();
}

let mut entities_per_system_per_class = EntitiesPerSystemPerClass::default();
entities_per_system_per_class
.entry("3D".into())
.or_default()
let mut entities_per_system = EntitiesPerSystem::default();

entities_per_system
.entry("Points3D".into())
.or_insert_with(|| {
[
Expand Down Expand Up @@ -758,7 +750,7 @@ mod tests {
),
};

let query_result = query.execute_query(&resolver, &ctx, &entities_per_system_per_class);
let query_result = query.execute_query(&resolver, &ctx, &entities_per_system);

let mut visited = vec![];
query_result.tree.visit(&mut |handle| {
Expand Down
44 changes: 26 additions & 18 deletions crates/re_viewer/src/app_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,21 @@ impl AppState {
.space_views
.values()
.flat_map(|space_view| {
space_view.queries.iter().map(|query| {
space_view.queries.iter().filter_map(|query| {
let props = viewport.state.space_view_props(space_view.id);
let resolver = query.build_resolver(space_view.id, props);
(
query.id,
query.execute_query(
&resolver,
store_context,
&entities_per_system_per_class,
),
)
entities_per_system_per_class
.get(&query.space_view_class_identifier)
.map(|entities_per_system| {
(
query.id,
query.execute_query(
&resolver,
store_context,
entities_per_system,
),
)
})
})
})
.collect::<_>()
Expand Down Expand Up @@ -194,17 +198,21 @@ impl AppState {
.space_views
.values()
.flat_map(|space_view| {
space_view.queries.iter().map(|query| {
space_view.queries.iter().filter_map(|query| {
let props = viewport.state.space_view_props(space_view.id);
let resolver = query.build_resolver(space_view.id, props);
(
query.id,
query.execute_query(
&resolver,
store_context,
&entities_per_system_per_class,
),
)
entities_per_system_per_class
.get(&query.space_view_class_identifier)
.map(|entities_per_system| {
(
query.id,
query.execute_query(
&resolver,
store_context,
entities_per_system,
),
)
})
})
})
.collect::<_>()
Expand Down
18 changes: 8 additions & 10 deletions crates/re_viewport/src/space_view.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ mod tests {
use re_log_types::{DataCell, DataRow, RowId, StoreId, TimePoint};
use re_space_view::DataQuery as _;
use re_types::archetypes::Points3D;
use re_viewer_context::{EntitiesPerSystemPerClass, StoreContext};
use re_viewer_context::{EntitiesPerSystem, StoreContext};

use super::*;

Expand Down Expand Up @@ -515,10 +515,8 @@ mod tests {

let auto_properties = Default::default();

let mut entities_per_system_per_class = EntitiesPerSystemPerClass::default();
entities_per_system_per_class
.entry("3D".into())
.or_default()
let mut entities_per_system = EntitiesPerSystem::default();
entities_per_system
.entry("Points3D".into())
.or_insert_with(|| {
[
Expand All @@ -541,7 +539,7 @@ mod tests {
all_recordings: vec![],
};

let query_result = query.execute_query(&resolver, &ctx, &entities_per_system_per_class);
let query_result = query.execute_query(&resolver, &ctx, &entities_per_system);

let parent = query_result
.tree
Expand Down Expand Up @@ -575,7 +573,7 @@ mod tests {
all_recordings: vec![],
};

let query_result = query.execute_query(&resolver, &ctx, &entities_per_system_per_class);
let query_result = query.execute_query(&resolver, &ctx, &entities_per_system);

let parent_group = query_result
.tree
Expand Down Expand Up @@ -618,7 +616,7 @@ mod tests {
all_recordings: vec![],
};

let query_result = query.execute_query(&resolver, &ctx, &entities_per_system_per_class);
let query_result = query.execute_query(&resolver, &ctx, &entities_per_system);

let parent = query_result
.tree
Expand Down Expand Up @@ -660,7 +658,7 @@ mod tests {
all_recordings: vec![],
};

let query_result = query.execute_query(&resolver, &ctx, &entities_per_system_per_class);
let query_result = query.execute_query(&resolver, &ctx, &entities_per_system);

let parent = query_result
.tree
Expand Down Expand Up @@ -697,7 +695,7 @@ mod tests {
all_recordings: vec![],
};

let query_result = query.execute_query(&resolver, &ctx, &entities_per_system_per_class);
let query_result = query.execute_query(&resolver, &ctx, &entities_per_system);

let parent = query_result
.tree
Expand Down
42 changes: 12 additions & 30 deletions crates/re_viewport/src/space_view_heuristics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +70,6 @@ pub fn all_possible_space_views(
}
}

let empty_entities_per_system = EntitiesPerSystem::default();

// Find all the entities that are used by the part (!) systems for each class.
// Note that entities_per_system_per_class includes both part-systems *and* context-systems
// so we filter out the context systems before aggregating the entities since context systems
// should not influence the heuristics.
let entities_used_by_any_part_system_of_class: IntMap<_, _> = ctx
.space_view_class_registry
.iter_registry()
.map(|entry| {
let class_identifier = entry.class.identifier();
let parts = ctx
.space_view_class_registry
.new_part_collection(class_identifier);
(
class_identifier,
entities_per_system_per_class
.get(&class_identifier)
.unwrap_or(&empty_entities_per_system)
.iter()
.filter(|(system, _)| parts.get_by_identifier(**system).is_ok())
.flat_map(|(_, entities)| entities.iter().cloned())
.collect::<IntSet<_>>(),
)
})
.collect();

// For each candidate, create space views for all possible classes.
candidate_space_view_paths(ctx, spaces_info)
.flat_map(|candidate_space_path| {
Expand All @@ -106,9 +79,18 @@ pub fn all_possible_space_views(
return Vec::new();
}

entities_used_by_any_part_system_of_class
entities_per_system_per_class
.iter()
.filter_map(|(class_identifier, _entities_used_by_any_part_system)| {
.filter_map(|(class_identifier, entities_per_system)| {
// We only want to run the query if at least one entity is reachable from the candidate.
if entities_per_system.values().all(|entities| {
!entities
.iter()
.any(|entity| entity.starts_with(candidate_space_path))
}) {
return None;
}

// TODO(#4377): The need to run a query-per-candidate for all possible candidates
// is way too expensive. This needs to be optimized significantly.
let candidate_query = DataQueryBlueprint::new(
Expand All @@ -119,7 +101,7 @@ pub fn all_possible_space_views(
let results = candidate_query.execute_query(
&NOOP_RESOLVER,
ctx.store_context,
entities_per_system_per_class,
entities_per_system,
);

if !results.is_empty() {
Expand Down

0 comments on commit 3f0cd4a

Please sign in to comment.