Skip to content

Commit

Permalink
Do query for default components only once per view (#8424)
Browse files Browse the repository at this point in the history
Moves query of default components to the "execute query" stage which we
perform for every view at the start of the frame.
Previously, we've done this as part of blueprint resolve on every 

Gives me about 2ms cpu frame time in the 2h airtraffic demo on my
windows machine running with single thread (since it's easier to
profile, `pixi run rerun-release --threads 1`) with the 3d view
maximized and the timline minimized.

Change is expected to have high impact for:
* many entities
* no or few transforms (because the transform cost shadows this win too
much and adds a lot of noise)

profile snapshots for said scenario:
before

![image](https://github.com/user-attachments/assets/009c00f9-d48b-4754-8cab-2539be87a2c1)

after:

![image](https://github.com/user-attachments/assets/3e5eb488-caec-49b4-937f-7bdea59a7b17)


Related future work:
* need short circuiting for overrides as well - we can just know when
there's none (and that's naturally very common!!)
* short circuit when there's no (fitting) default components?
* reduce the amount of query objects being passed around, we have too
many of those at this point!
  • Loading branch information
Wumpf authored Dec 12, 2024
1 parent 2046397 commit 71afae7
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 65 deletions.
2 changes: 1 addition & 1 deletion crates/store/re_query/src/latest_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl QueryCache {
///
/// Use [`LatestAtResults::get`] and/or [`LatestAtResults::get_required`] in order to access
/// the results for each individual component.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct LatestAtResults {
/// The associated [`EntityPath`].
pub entity_path: EntityPath,
Expand Down
12 changes: 6 additions & 6 deletions crates/viewer/re_selection_panel/src/visualizer_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,19 +246,19 @@ fn visualizer_components(
ValueSource::Override => (
ctx.viewer_ctx.blueprint_query,
ctx.blueprint_db(),
override_path,
override_path.clone(),
result_override.unwrap(),
),
ValueSource::Store => (
&store_query,
ctx.recording(),
&data_result.entity_path,
data_result.entity_path.clone(),
result_store.unwrap(),
),
ValueSource::Default => (
ctx.viewer_ctx.blueprint_query,
ctx.blueprint_db(),
ctx.defaults_path,
ViewBlueprint::defaults_path(ctx.view_id),
result_default.unwrap(),
),
ValueSource::FallbackOrPlaceholder => {
Expand All @@ -280,7 +280,7 @@ fn visualizer_components(
};

re_data_ui::ComponentPathLatestAtResults {
component_path: ComponentPath::new(entity_path.clone(), component_name),
component_path: ComponentPath::new(entity_path, component_name),
unit: latest_at_unit,
}
.data_ui(ctx.viewer_ctx, ui, UiLayout::List, query, db);
Expand Down Expand Up @@ -340,7 +340,7 @@ fn visualizer_components(
&query_ctx,
ui,
"Default",
ctx.defaults_path,
&ViewBlueprint::defaults_path(ctx.view_id),
component_name,
*row_id,
raw_default.as_ref(),
Expand Down Expand Up @@ -501,7 +501,7 @@ fn menu_more(

if ui.button("Make default for current view").clicked() {
ctx.save_blueprint_array(
ctx.defaults_path,
&ViewBlueprint::defaults_path(ctx.view_id),
component_name,
raw_current_value.to_boxed(),
);
Expand Down
36 changes: 8 additions & 28 deletions crates/viewer/re_view/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ use re_viewer_context::{
///
/// Data should be accessed via the [`crate::RangeResultsExt`] trait which is implemented for
/// [`crate::HybridResults`].
pub fn range_with_blueprint_resolved_data(
ctx: &ViewContext<'_>,
pub fn range_with_blueprint_resolved_data<'a>(
ctx: &ViewContext<'a>,
_annotations: Option<&re_viewer_context::Annotations>,
range_query: &RangeQuery,
data_result: &re_viewer_context::DataResult,
component_names: impl IntoIterator<Item = ComponentName>,
) -> HybridRangeResults {
) -> HybridRangeResults<'a> {
re_tracing::profile_function!(data_result.entity_path.to_string());

let mut component_name_set = component_names.into_iter().collect::<IntSet<_>>();
Expand All @@ -47,20 +47,10 @@ pub fn range_with_blueprint_resolved_data(
component_name_set.iter(),
);

// TODO(jleibs): This doesn't work when the component set contains empty results.
// This means we over-query for defaults that will never be used.
// component_set.retain(|component| !results.components.contains_key(component));

let defaults = ctx.viewer_ctx.blueprint_engine().cache().latest_at(
ctx.viewer_ctx.blueprint_query,
ctx.defaults_path,
component_name_set.iter().copied(),
);

HybridRangeResults {
overrides,
results,
defaults,
defaults: &ctx.query_result.component_defaults,
}
}

Expand All @@ -76,14 +66,14 @@ pub fn range_with_blueprint_resolved_data(
/// Data should be accessed via the [`crate::RangeResultsExt`] trait which is implemented for
/// [`crate::HybridResults`].
///
/// If `query_shadowed_defaults` is true, all defaults will be queried, even if they are not used.
/// If `query_shadowed_components` is true, store components will be queried, even if they are not used.
pub fn latest_at_with_blueprint_resolved_data<'a>(
ctx: &'a ViewContext<'a>,
_annotations: Option<&'a re_viewer_context::Annotations>,
latest_at_query: &LatestAtQuery,
data_result: &'a re_viewer_context::DataResult,
component_names: impl IntoIterator<Item = ComponentName>,
query_shadowed_defaults: bool,
query_shadowed_components: bool,
) -> HybridLatestAtResults<'a> {
// This is called very frequently, don't put a profile scope here.

Expand All @@ -92,7 +82,7 @@ pub fn latest_at_with_blueprint_resolved_data<'a>(
let overrides = query_overrides(ctx.viewer_ctx, data_result, component_set.iter());

// No need to query for components that have overrides unless opted in!
if !query_shadowed_defaults {
if !query_shadowed_components {
component_set.retain(|component| !overrides.components.contains_key(component));
}

Expand All @@ -102,20 +92,10 @@ pub fn latest_at_with_blueprint_resolved_data<'a>(
component_set.iter().copied(),
);

// TODO(jleibs): This doesn't work when the component set contains empty results.
// This means we over-query for defaults that will never be used.
// component_set.retain(|component| !results.components.contains_key(component));

let defaults = ctx.viewer_ctx.blueprint_engine().cache().latest_at(
ctx.viewer_ctx.blueprint_query,
ctx.defaults_path,
component_set.iter().copied(),
);

HybridLatestAtResults {
overrides,
results,
defaults,
defaults: &ctx.query_result.component_defaults,
ctx,
query: latest_at_query.clone(),
data_result,
Expand Down
24 changes: 11 additions & 13 deletions crates/viewer/re_view/src/results_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::DataResultQuery as _;
pub struct HybridLatestAtResults<'a> {
pub overrides: LatestAtResults,
pub results: LatestAtResults,
pub defaults: LatestAtResults,
pub defaults: &'a LatestAtResults,

pub ctx: &'a ViewContext<'a>,
pub query: LatestAtQuery,
Expand All @@ -33,10 +33,10 @@ pub struct HybridLatestAtResults<'a> {
/// Although overrides are never temporal, when accessed via the [`crate::RangeResultsExt`] trait
/// they will be merged into the results appropriately.
#[derive(Debug)]
pub struct HybridRangeResults {
pub struct HybridRangeResults<'a> {
pub(crate) overrides: LatestAtResults,
pub(crate) results: RangeResults,
pub(crate) defaults: LatestAtResults,
pub(crate) defaults: &'a LatestAtResults,
}

impl HybridLatestAtResults<'_> {
Expand Down Expand Up @@ -130,7 +130,7 @@ pub enum HybridResults<'a> {
LatestAt(LatestAtQuery, HybridLatestAtResults<'a>),

// Boxed because of size difference between variants
Range(RangeQuery, Box<HybridRangeResults>),
Range(RangeQuery, Box<HybridRangeResults<'a>>),
}

impl HybridResults<'_> {
Expand All @@ -141,9 +141,8 @@ impl HybridResults<'_> {
match self {
Self::LatestAt(_, r) => {
let mut indices = Vec::with_capacity(
r.defaults.components.len()
+ r.overrides.components.len()
+ r.results.components.len(),
// Don't add defaults component count because that's defaults for the entire view.
r.overrides.components.len() + r.results.components.len(),
);

indices.extend(
Expand All @@ -170,9 +169,8 @@ impl HybridResults<'_> {

Self::Range(_, r) => {
let mut indices = Vec::with_capacity(
r.defaults.components.len()
+ r.overrides.components.len()
+ r.results.components.len(), // Don't know how many results per component.
// Don't add defaults component count because that's defaults for the entire view.
r.overrides.components.len() + r.results.components.len(),
);

indices.extend(
Expand Down Expand Up @@ -213,9 +211,9 @@ impl<'a> From<(LatestAtQuery, HybridLatestAtResults<'a>)> for HybridResults<'a>
}
}

impl From<(RangeQuery, HybridRangeResults)> for HybridResults<'_> {
impl<'a> From<(RangeQuery, HybridRangeResults<'a>)> for HybridResults<'a> {
#[inline]
fn from((query, results): (RangeQuery, HybridRangeResults)) -> Self {
fn from((query, results): (RangeQuery, HybridRangeResults<'a>)) -> Self {
Self::Range(query, Box::new(results))
}
}
Expand Down Expand Up @@ -288,7 +286,7 @@ impl RangeResultsExt for RangeResults {
}
}

impl RangeResultsExt for HybridRangeResults {
impl<'a> RangeResultsExt for HybridRangeResults<'a> {
#[inline]
fn get_required_chunks(&self, component_name: &ComponentName) -> Option<Cow<'_, [Chunk]>> {
if let Some(unit) = self.overrides.get(component_name) {
Expand Down
9 changes: 7 additions & 2 deletions crates/viewer/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,13 @@ impl AppState {

(
view.id,
view.contents
.execute_query(store_context, &visualizable_entities),
view.contents.execute_query(
store_context,
view_class_registry,
&blueprint_query,
view.id,
&visualizable_entities,
),
)
})
.collect::<_>()
Expand Down
25 changes: 22 additions & 3 deletions crates/viewer/re_viewer_context/src/query_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use smallvec::SmallVec;

use re_log_types::{EntityPath, EntityPathHash};

use crate::{DataResult, ViewContext, ViewId, ViewState, ViewerContext};
use crate::{blueprint_timeline, DataResult, ViewContext, ViewId, ViewState, ViewerContext};

slotmap::new_key_type! {
/// Identifier for a [`DataResultNode`]
Expand Down Expand Up @@ -48,8 +48,7 @@ impl QueryContext<'_> {
}
}

/// The result of executing a single data query
#[derive(Default)]
/// The result of executing a single data query for a specific view.
pub struct DataQueryResult {
/// The [`DataResultTree`] for the query
pub tree: DataResultTree,
Expand All @@ -62,6 +61,25 @@ pub struct DataQueryResult {
/// This does *not* take into account the actual selection of visualizers
/// which may be an explicit none for any given entity.
pub num_visualized_entities: usize,

/// Latest-at results for all component defaults in this view.
pub component_defaults: re_query::LatestAtResults,
}

impl Default for DataQueryResult {
fn default() -> Self {
Self {
tree: Default::default(),
num_matching_entities: 0,
num_visualized_entities: 0,
component_defaults: re_query::LatestAtResults {
entity_path: "<defaults>".into(),
query: re_chunk_store::LatestAtQuery::latest(blueprint_timeline()),
compound_index: (re_chunk::TimeInt::STATIC, re_chunk::RowId::ZERO),
components: Default::default(),
},
}
}
}

impl DataQueryResult {
Expand All @@ -85,6 +103,7 @@ impl Clone for DataQueryResult {
tree: self.tree.clone(),
num_matching_entities: self.num_matching_entities,
num_visualized_entities: self.num_visualized_entities,
component_defaults: self.component_defaults.clone(),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_viewer_context/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub struct TestContext {
pub selection_state: ApplicationSelectionState,
pub recording_config: RecordingConfig,

blueprint_query: LatestAtQuery,
pub blueprint_query: LatestAtQuery,
component_ui_registry: ComponentUiRegistry,

command_sender: CommandSender,
Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_viewer_context/src/view/view_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ pub struct ViewContext<'a> {
pub viewer_ctx: &'a crate::ViewerContext<'a>,
pub view_id: ViewId,
pub view_state: &'a dyn crate::ViewState,
pub defaults_path: &'a EntityPath,
pub visualizer_collection: Arc<crate::VisualizerCollection>,
pub query_result: &'a DataQueryResult,
}

impl<'a> ViewContext<'a> {
Expand Down
16 changes: 11 additions & 5 deletions crates/viewer/re_viewport_blueprint/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,8 @@ impl ViewBlueprint {
viewer_ctx: ctx,
view_id: self.id,
view_state,
defaults_path: &self.defaults_path,
visualizer_collection: self.visualizer_collection(ctx),
query_result: ctx.lookup_query_result(self.id),
}
}

Expand All @@ -414,8 +414,8 @@ impl ViewBlueprint {
viewer_ctx: ctx,
view_id: self.id,
view_state,
defaults_path: &self.defaults_path,
visualizer_collection: self.visualizer_collection(ctx),
query_result: ctx.lookup_query_result(self.id),
}
}

Expand Down Expand Up @@ -715,7 +715,7 @@ mod tests {

// Set up a store query and update the overrides.
let query_result =
update_overrides(&test_ctx, &view.contents, &visualizable_entities, &resolver);
update_overrides(&test_ctx, &view, &visualizable_entities, &resolver);

// Extract component overrides for testing.
let mut visited: HashMap<EntityPath, HashMap<ComponentName, EntityPath>> =
Expand Down Expand Up @@ -745,7 +745,7 @@ mod tests {

fn update_overrides(
test_ctx: &TestContext,
contents: &ViewContents,
view: &ViewBlueprint,
visualizable_entities: &PerVisualizer<VisualizableEntities>,
resolver: &DataQueryPropertyResolver<'_>,
) -> re_viewer_context::DataQueryResult {
Expand All @@ -759,7 +759,13 @@ mod tests {
hub: &re_viewer_context::StoreHub::test_hub(),
};

let mut query_result = contents.execute_query(&store_ctx, visualizable_entities);
let mut query_result = view.contents.execute_query(
&store_ctx,
&test_ctx.view_class_registry,
&test_ctx.blueprint_query,
view.id,
visualizable_entities,
);
let mut view_states = ViewStates::default();

test_ctx.run_in_egui_central_panel(|ctx, _ui| {
Expand Down
Loading

0 comments on commit 71afae7

Please sign in to comment.