Skip to content

Commit

Permalink
Improve performance for scenes with many entities & transforms (#7456)
Browse files Browse the repository at this point in the history
### What

Not super significant unfortunately, in one badly affected scene I went
from ~130ms to ~110ms. It's probably better in those that aren't using
any transforms at all.
We need to do some more general overhaul there and imho at that point we
might as well start thinking more seriously about range queries for
transforms. See among others:
* #7025
* #723

### 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 examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7456?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7456?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
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide
* [x] run checklists & samples to check if transforms still work fine

- [PR Build Summary](https://build.rerun.io/pr/7456)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
Wumpf authored Sep 24, 2024
1 parent 199f48a commit 7c7c712
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 65 deletions.
8 changes: 4 additions & 4 deletions crates/store/re_chunk_store/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,8 @@ impl ChunkStore {
entity_path: &EntityPath,
component_name: ComponentName,
) -> Vec<Arc<Chunk>> {
re_tracing::profile_function!(format!("{query:?}"));

self.query_id.fetch_add(1, Ordering::Relaxed);
// Don't do a profile scope here, this can have a lot of overhead when executing many small queries.
//re_tracing::profile_function!(format!("{query:?}"));

// Reminder: if a chunk has been indexed for a given component, then it must contain at
// least one non-null value for that column.
Expand Down Expand Up @@ -510,7 +509,8 @@ impl ChunkStore {
query: &LatestAtQuery,
temporal_chunk_ids_per_time: &ChunkIdSetPerTime,
) -> Option<Vec<Arc<Chunk>>> {
re_tracing::profile_function!();
// Don't do a profile scope here, this can have a lot of overhead when executing many small queries.
//re_tracing::profile_function!();

let upper_bound = temporal_chunk_ids_per_time
.per_start_time
Expand Down
3 changes: 2 additions & 1 deletion crates/store/re_query/src/latest_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,8 @@ impl LatestAtCache {
entity_path: &EntityPath,
component_name: ComponentName,
) -> Option<UnitChunkShared> {
re_tracing::profile_scope!("latest_at", format!("{component_name} @ {query:?}"));
// Don't do a profile scope here, this can have a lot of overhead when executing many small queries.
//re_tracing::profile_scope!("latest_at", format!("{component_name} @ {query:?}"));

debug_assert_eq!(query.timeline(), self.cache_key.timeline);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -529,16 +529,8 @@ fn query_and_resolve_tree_transform_at_entity(
entity_path: &EntityPath,
entity_db: &EntityDb,
query: &LatestAtQuery,
transform3d_components: impl Iterator<Item = re_types::ComponentName>,
) -> Option<glam::Affine3A> {
let Some(transform3d_components) =
TransformComponentTracker::access(entity_db.store_id(), |tracker| {
tracker.transform3d_components(entity_path).cloned()
})
.flatten()
else {
return None;
};

// TODO(#6743): Doesn't take into account overrides.
let result = entity_db.latest_at(query, entity_path, transform3d_components);
if result.components.is_empty() {
Expand Down Expand Up @@ -575,16 +567,8 @@ fn query_and_resolve_instance_poses_at_entity(
entity_path: &EntityPath,
entity_db: &EntityDb,
query: &LatestAtQuery,
pose3d_components: impl Iterator<Item = re_types::ComponentName>,
) -> Vec<glam::Affine3A> {
let Some(pose3d_components) =
TransformComponentTracker::access(entity_db.store_id(), |tracker| {
tracker.pose3d_components(entity_path).cloned()
})
.flatten()
else {
return Vec::new();
};

// TODO(#6743): Doesn't take into account overrides.
let result = entity_db.latest_at(query, entity_path, pose3d_components);

Expand Down Expand Up @@ -726,6 +710,7 @@ fn query_and_resolve_obj_from_pinhole_image_plane(
}

/// Resolved transforms at an entity.
#[derive(Default)]
struct TransformsAtEntity {
parent_from_entity_tree_transform: Option<glam::Affine3A>,
entity_from_instance_poses: Vec<glam::Affine3A>,
Expand All @@ -741,23 +726,49 @@ fn transforms_at(
) -> Result<TransformsAtEntity, UnreachableTransformReason> {
re_tracing::profile_function!();

let transforms_at_entity = TransformsAtEntity {
parent_from_entity_tree_transform: query_and_resolve_tree_transform_at_entity(
let potential_transform_components =
TransformComponentTracker::access(entity_db.store_id(), |tracker| {
tracker.potential_transform_components(entity_path).cloned()
})
.flatten()
.unwrap_or_default();

let parent_from_entity_tree_transform = if potential_transform_components.transform3d.is_empty()
{
None
} else {
query_and_resolve_tree_transform_at_entity(
entity_path,
entity_db,
query,
),
entity_from_instance_poses: query_and_resolve_instance_poses_at_entity(
potential_transform_components.transform3d.iter().copied(),
)
};
let entity_from_instance_poses = if potential_transform_components.pose3d.is_empty() {
Vec::new()
} else {
query_and_resolve_instance_poses_at_entity(
entity_path,
entity_db,
query,
),
instance_from_pinhole_image_plane: query_and_resolve_obj_from_pinhole_image_plane(
potential_transform_components.pose3d.iter().copied(),
)
};
let instance_from_pinhole_image_plane = if potential_transform_components.pinhole {
query_and_resolve_obj_from_pinhole_image_plane(
entity_path,
entity_db,
query,
pinhole_image_plane_distance,
),
)
} else {
None
};

let transforms_at_entity = TransformsAtEntity {
parent_from_entity_tree_transform,
entity_from_instance_poses,
instance_from_pinhole_image_plane,
};

// Handle pinhole encounters.
Expand All @@ -773,13 +784,16 @@ fn transforms_at(
}

// If there is any other transform, we ignore `DisconnectedSpace`.
if transforms_at_entity
let no_other_transforms = transforms_at_entity
.parent_from_entity_tree_transform
.is_none()
&& transforms_at_entity.entity_from_instance_poses.is_empty()
&& transforms_at_entity
.instance_from_pinhole_image_plane
.is_none()
.is_none();

if no_other_transforms
&& potential_transform_components.disconnected_space
&& entity_db
.latest_at_component::<DisconnectedSpace>(entity_path, query)
.map_or(false, |(_index, res)| **res)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,26 @@ use re_chunk_store::{
ChunkStoreSubscriberHandle,
};
use re_log_types::{EntityPath, EntityPathHash, StoreId};
use re_types::ComponentName;
use re_types::{ComponentName, Loggable as _};

// ---

/// Set of components that an entity ever had over its known lifetime.
#[derive(Default, Clone)]
pub struct PotentialTransformComponentSet {
/// All transform components ever present.
pub transform3d: IntSet<ComponentName>,

/// All pose transform components ever present.
pub pose3d: IntSet<ComponentName>,

/// Whether the entity ever had a pinhole camera.
pub pinhole: bool,

/// Whether the entity ever had a disconnected space component.
pub disconnected_space: bool,
}

/// Keeps track of which entities have had any `Transform3D`-related data on any timeline at any
/// point in time.
///
Expand All @@ -20,13 +36,7 @@ use re_types::ComponentName;
/// This is a huge performance improvement in practice, especially in recordings with many entities.
#[derive(Default)]
pub struct TransformComponentTracker {
/// Which entities have had any `Transform3D` component at any point in time, and which
/// components they actually make use of.
transform3d_entities: IntMap<EntityPathHash, IntSet<ComponentName>>,

/// Which entities have had any `InstancePoses3D` components at any point in time, and
/// which components they actually make use of.
pose3d_entities: IntMap<EntityPathHash, IntSet<ComponentName>>,
components_per_entity: IntMap<EntityPathHash, PotentialTransformComponentSet>,
}

impl TransformComponentTracker {
Expand All @@ -42,17 +52,11 @@ impl TransformComponentTracker {
.flatten()
}

#[inline]
pub fn transform3d_components(
pub fn potential_transform_components(
&self,
entity_path: &EntityPath,
) -> Option<&IntSet<ComponentName>> {
self.transform3d_entities.get(&entity_path.hash())
}

#[inline]
pub fn pose3d_components(&self, entity_path: &EntityPath) -> Option<&IntSet<ComponentName>> {
self.pose3d_entities.get(&entity_path.hash())
) -> Option<&PotentialTransformComponentSet> {
self.components_per_entity.get(&entity_path.hash())
}
}

Expand Down Expand Up @@ -123,37 +127,55 @@ impl ChunkStoreSubscriber for TransformComponentTrackerStoreSubscriber {

let entity_path_hash = event.chunk.entity_path().hash();

let contains_non_zero_component_array = |component_name| {
event
.chunk
.components()
.get(&component_name)
.map_or(false, |list_array| {
list_array.offsets().lengths().any(|len| len > 0)
})
};

for component_name in event.chunk.component_names() {
if self.transform_components.contains(&component_name)
&& event
.chunk
.components()
.get(&component_name)
.map_or(false, |list_array| {
list_array.offsets().lengths().any(|len| len > 0)
})
&& contains_non_zero_component_array(component_name)
{
transform_component_tracker
.transform3d_entities
.components_per_entity
.entry(entity_path_hash)
.or_default()
.transform3d
.insert(component_name);
}
if self.pose_components.contains(&component_name)
&& event
.chunk
.components()
.get(&component_name)
.map_or(false, |list_array| {
list_array.offsets().lengths().any(|len| len > 0)
})
&& contains_non_zero_component_array(component_name)
{
transform_component_tracker
.pose3d_entities
.components_per_entity
.entry(entity_path_hash)
.or_default()
.pose3d
.insert(component_name);
}
if component_name == re_types::components::PinholeProjection::name()
&& contains_non_zero_component_array(component_name)
{
transform_component_tracker
.components_per_entity
.entry(entity_path_hash)
.or_default()
.pinhole = true;
}
if component_name == re_types::components::DisconnectedSpace::name()
&& contains_non_zero_component_array(component_name)
{
transform_component_tracker
.components_per_entity
.entry(entity_path_hash)
.or_default()
.disconnected_space = true;
}
}
}
}
Expand Down

0 comments on commit 7c7c712

Please sign in to comment.