Skip to content

Commit

Permalink
don't copy default query result around
Browse files Browse the repository at this point in the history
  • Loading branch information
Wumpf committed Dec 11, 2024
1 parent 5848e24 commit 1125a7d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 31 deletions.
22 changes: 5 additions & 17 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,16 +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.query_result.component_defaults.clone(); // TODO(andreas): don't clone

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

Expand Down Expand Up @@ -98,16 +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.query_result.component_defaults.clone(); // TODO(andreas): don't clone

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
2 changes: 1 addition & 1 deletion crates/viewer/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl AppState {
view.id,
view.contents.execute_query(
store_context,
&view_class_registry,
view_class_registry,
&blueprint_query,
view.id,
&visualizable_entities,
Expand Down

0 comments on commit 1125a7d

Please sign in to comment.