Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance regression when viewing images and tensors #3767

Merged
merged 7 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/re_arrow_store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub mod test_util;
pub use self::arrow_util::ArrayExt;
pub use self::store::{DataStore, DataStoreConfig, StoreGeneration};
pub use self::store_gc::{Deleted, GarbageCollectionOptions, GarbageCollectionTarget};
pub use self::store_helpers::VersionedComponent;
pub use self::store_read::{LatestAtQuery, RangeQuery};
pub use self::store_stats::{DataStoreRowStats, DataStoreStats, EntityStats};
pub use self::store_write::{WriteError, WriteResult};
Expand Down
17 changes: 16 additions & 1 deletion crates/re_data_store/src/instance_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ fn test_parse_instance_path() {
/// Hashes of the components of an [`InstancePath`].
///
/// This is unique to either a specific instance of an entity, or the whole entity (splat).
#[derive(Clone, Copy, Debug, Eq)]
#[derive(Clone, Copy, Eq)]
pub struct InstancePathHash {
pub entity_path_hash: EntityPathHash,

Expand All @@ -135,6 +135,21 @@ pub struct InstancePathHash {
pub instance_key: InstanceKey,
}

impl std::fmt::Debug for InstancePathHash {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let Self {
entity_path_hash,
instance_key,
} = self;
write!(
f,
"InstancePathHash({:016X}, {})",
entity_path_hash.hash64(),
instance_key.0
)
}
}

impl std::hash::Hash for InstancePathHash {
#[inline]
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
Expand Down
15 changes: 14 additions & 1 deletion crates/re_data_store/src/versioned_instance_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,25 @@ impl std::fmt::Display for VersionedInstancePath {
///
/// The easiest way to construct this type is to use either [`crate::InstancePathHash::versioned`]
/// or [`crate::VersionedInstancePath::hash`].
#[derive(Clone, Copy, Debug, Eq)]
#[derive(Clone, Copy, Eq)]
pub struct VersionedInstancePathHash {
pub instance_path_hash: InstancePathHash,
pub row_id: RowId,
}

impl std::fmt::Debug for VersionedInstancePathHash {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let Self {
instance_path_hash,
row_id,
} = self;
write!(
f,
"VersionedInstancePathHash({instance_path_hash:?}, {row_id})"
)
}
}

impl std::hash::Hash for VersionedInstancePathHash {
#[inline]
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
Expand Down
29 changes: 13 additions & 16 deletions crates/re_data_ui/src/image.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use egui::{Color32, Vec2};
use itertools::Itertools as _;

use re_data_store::{InstancePathHash, VersionedInstancePathHash};
use re_log_types::RowId;
use re_renderer::renderer::ColormappedTexture;
use re_types::components::{ClassId, DepthMeter};
Expand Down Expand Up @@ -32,18 +31,16 @@ impl EntityDataUi for re_types::components::TensorData {
) {
re_tracing::profile_function!();

let row_id = ctx
let tensor_data_row_id = ctx
.store_db
.entity_db
.data_store
.query_latest_component::<re_types::components::TensorData>(entity_path, query)
.map_or(RowId::ZERO, |tensor| tensor.row_id);

// NOTE: Tensors don't support batches at the moment so always splat.
let tensor_path_hash = InstancePathHash::entity_splat(entity_path).versioned(row_id);
let decoded = ctx
.cache
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_path_hash, self.0.clone()));
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_data_row_id, self.0.clone()));
match decoded {
Ok(decoded) => {
let annotations = crate::annotations(ctx, query, entity_path);
Expand All @@ -53,7 +50,7 @@ impl EntityDataUi for re_types::components::TensorData {
verbosity,
entity_path,
&annotations,
tensor_path_hash,
tensor_data_row_id,
&self.0,
&decoded,
);
Expand All @@ -72,15 +69,15 @@ fn tensor_ui(
verbosity: UiVerbosity,
entity_path: &re_data_store::EntityPath,
annotations: &Annotations,
tensor_path_hash: VersionedInstancePathHash,
tensor_data_row_id: RowId,
original_tensor: &TensorData,
tensor: &DecodedTensor,
) {
// See if we can convert the tensor to a GPU texture.
// Even if not, we will show info about the tensor.
let tensor_stats = ctx
.cache
.entry(|c: &mut TensorStatsCache| c.entry(tensor_path_hash, tensor));
.entry(|c: &mut TensorStatsCache| c.entry(tensor_data_row_id, tensor));
let debug_name = entity_path.to_string();

let meaning = image_meaning_for_entity(entity_path, ctx);
Expand All @@ -97,7 +94,7 @@ fn tensor_ui(
let texture_result = gpu_bridge::tensor_to_gpu(
ctx.render_ctx,
&debug_name,
tensor_path_hash,
tensor_data_row_id,
tensor,
meaning,
&tensor_stats,
Expand Down Expand Up @@ -199,7 +196,7 @@ fn tensor_ui(
ctx.render_ctx,
ui,
response,
tensor_path_hash,
tensor_data_row_id,
tensor,
&tensor_stats,
annotations,
Expand Down Expand Up @@ -423,7 +420,7 @@ fn show_zoomed_image_region_tooltip(
render_ctx: &mut re_renderer::RenderContext,
parent_ui: &egui::Ui,
response: egui::Response,
tensor_path_hash: VersionedInstancePathHash,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
tensor_stats: &TensorStats,
annotations: &Annotations,
Expand Down Expand Up @@ -456,7 +453,7 @@ fn show_zoomed_image_region_tooltip(
show_zoomed_image_region(
render_ctx,
ui,
tensor_path_hash,
tensor_data_row_id,
tensor,
tensor_stats,
annotations,
Expand Down Expand Up @@ -512,7 +509,7 @@ pub fn show_zoomed_image_region_area_outline(
pub fn show_zoomed_image_region(
render_ctx: &mut re_renderer::RenderContext,
ui: &mut egui::Ui,
tensor_path_hash: VersionedInstancePathHash,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
tensor_stats: &TensorStats,
annotations: &Annotations,
Expand All @@ -524,7 +521,7 @@ pub fn show_zoomed_image_region(
if let Err(err) = try_show_zoomed_image_region(
render_ctx,
ui,
tensor_path_hash,
tensor_data_row_id,
tensor,
tensor_stats,
annotations,
Expand All @@ -542,7 +539,7 @@ pub fn show_zoomed_image_region(
fn try_show_zoomed_image_region(
render_ctx: &mut re_renderer::RenderContext,
ui: &mut egui::Ui,
tensor_path_hash: VersionedInstancePathHash,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
tensor_stats: &TensorStats,
annotations: &Annotations,
Expand All @@ -558,7 +555,7 @@ fn try_show_zoomed_image_region(
let texture = gpu_bridge::tensor_to_gpu(
render_ctx,
debug_name,
tensor_path_hash,
tensor_data_row_id,
tensor,
meaning,
tensor_stats,
Expand Down
6 changes: 2 additions & 4 deletions crates/re_renderer/src/resource_managers/texture_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,8 @@ pub struct TextureManager2D {

#[derive(Default)]
struct Inner {
/// Caches textures using a `VersionedInstancePathHash`, i.e. a specific instance of a specific
/// entity path for a specific row in the store.
///
/// For dependency reasons, the versioned path has to be provided pre-hashed as a u64 by the user.
/// Caches textures using a unique id, which in practice is the hash of the
/// row id of the tensor data (`tensor_data_row_id`).
///
/// Any texture which wasn't accessed on the previous frame is ejected from the cache
/// during [`Self::begin_frame`].
Expand Down
45 changes: 21 additions & 24 deletions crates/re_space_view_spatial/src/parts/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use itertools::Itertools as _;
use nohash_hasher::IntSet;

use re_arrow_store::LatestAtQuery;
use re_data_store::{EntityPath, EntityProperties, InstancePathHash, VersionedInstancePathHash};
use re_log_types::EntityPathHash;
use re_data_store::{EntityPath, EntityProperties};
use re_log_types::{EntityPathHash, RowId};
use re_query::{ArchetypeView, QueryError};
use re_renderer::{
renderer::{DepthCloud, DepthClouds, RectangleOptions, TexturedRect},
Expand Down Expand Up @@ -57,7 +57,7 @@ fn to_textured_rect(
ctx: &ViewerContext<'_>,
ent_path: &EntityPath,
ent_context: &SpatialSceneEntityContext<'_>,
tensor_path_hash: VersionedInstancePathHash,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
meaning: TensorDataMeaning,
multiplicative_tint: egui::Rgba,
Expand All @@ -71,12 +71,12 @@ fn to_textured_rect(
let debug_name = ent_path.to_string();
let tensor_stats = ctx
.cache
.entry(|c: &mut TensorStatsCache| c.entry(tensor_path_hash, tensor));
.entry(|c: &mut TensorStatsCache| c.entry(tensor_data_row_id, tensor));

match gpu_bridge::tensor_to_gpu(
ctx.render_ctx,
&debug_name,
tensor_path_hash,
tensor_data_row_id,
tensor,
meaning,
&tensor_stats,
Expand Down Expand Up @@ -239,12 +239,11 @@ impl ImagesPart {
return Ok(());
}

// NOTE: Tensors don't support batches at the moment so always splat.
let tensor_path_hash =
InstancePathHash::entity_splat(ent_path).versioned(arch_view.primary_row_id());
let tensor_data_row_id = arch_view.primary_row_id();

let tensor = match ctx
.cache
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_path_hash, tensor.0))
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_data_row_id, tensor.0))
{
Ok(tensor) => tensor,
Err(err) => {
Expand All @@ -268,7 +267,7 @@ impl ImagesPart {
ctx,
ent_path,
ent_context,
tensor_path_hash,
tensor_data_row_id,
&tensor,
meaning,
color.into(),
Expand Down Expand Up @@ -330,12 +329,11 @@ impl ImagesPart {
return Ok(());
}

// NOTE: Tensors don't support batches at the moment so always splat.
let tensor_path_hash =
InstancePathHash::entity_splat(ent_path).versioned(arch_view.primary_row_id());
let tensor_data_row_id = arch_view.primary_row_id();

let tensor = match ctx
.cache
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_path_hash, tensor.0))
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_data_row_id, tensor.0))
{
Ok(tensor) => tensor,
Err(err) => {
Expand All @@ -356,7 +354,7 @@ impl ImagesPart {
transforms,
ent_context,
ent_props,
tensor_path_hash,
tensor_data_row_id,
&tensor,
ent_path,
parent_pinhole_path,
Expand Down Expand Up @@ -388,7 +386,7 @@ impl ImagesPart {
ctx,
ent_path,
ent_context,
tensor_path_hash,
tensor_data_row_id,
&tensor,
meaning,
color.into(),
Expand Down Expand Up @@ -447,12 +445,11 @@ impl ImagesPart {
return Ok(());
}

// NOTE: Tensors don't support batches at the moment so always splat.
let tensor_path_hash =
InstancePathHash::entity_splat(ent_path).versioned(arch_view.primary_row_id());
let tensor_data_row_id = arch_view.primary_row_id();

let tensor = match ctx
.cache
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_path_hash, tensor.0))
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_data_row_id, tensor.0))
{
Ok(tensor) => tensor,
Err(err) => {
Expand All @@ -476,7 +473,7 @@ impl ImagesPart {
ctx,
ent_path,
ent_context,
tensor_path_hash,
tensor_data_row_id,
&tensor,
meaning,
color.into(),
Expand All @@ -503,7 +500,7 @@ impl ImagesPart {
transforms: &TransformContext,
ent_context: &SpatialSceneEntityContext<'_>,
properties: &EntityProperties,
tensor_path_hash: VersionedInstancePathHash,
tensor_data_row_id: RowId,
tensor: &DecodedTensor,
ent_path: &EntityPath,
parent_pinhole_path: &EntityPath,
Expand Down Expand Up @@ -543,11 +540,11 @@ impl ImagesPart {
let debug_name = ent_path.to_string();
let tensor_stats = ctx
.cache
.entry(|c: &mut TensorStatsCache| c.entry(tensor_path_hash, tensor));
.entry(|c: &mut TensorStatsCache| c.entry(tensor_data_row_id, tensor));
let depth_texture = re_viewer_context::gpu_bridge::depth_tensor_to_gpu(
ctx.render_ctx,
&debug_name,
tensor_path_hash,
tensor_data_row_id,
tensor,
&tensor_stats,
)?;
Expand Down
10 changes: 5 additions & 5 deletions crates/re_space_view_spatial/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ pub fn picking(
ui_clip_rect,
coords,
space_from_ui,
tensor_path_hash,
tensor_path_hash.row_id,
annotations,
meaning,
meter,
Expand Down Expand Up @@ -682,7 +682,7 @@ fn image_hover_ui(
ui_clip_rect: egui::Rect,
coords: [u32; 2],
space_from_ui: egui::emath::RectTransform,
tensor_path_hash: re_data_store::VersionedInstancePathHash,
tensor_data_row_id: re_log_types::RowId,
annotations: &AnnotationSceneContext,
meaning: TensorDataMeaning,
meter: Option<f32>,
Expand Down Expand Up @@ -720,17 +720,17 @@ fn image_hover_ui(

let decoded_tensor = ctx
.cache
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_path_hash, tensor.0));
.entry(|c: &mut TensorDecodeCache| c.entry(tensor_data_row_id, tensor.0));
match decoded_tensor {
Ok(decoded_tensor) => {
let annotations = annotations.0.find(&instance_path.entity_path);
let tensor_stats = ctx.cache.entry(|c: &mut TensorStatsCache| {
c.entry(tensor_path_hash, &decoded_tensor)
c.entry(tensor_data_row_id, &decoded_tensor)
});
show_zoomed_image_region(
ctx.render_ctx,
ui,
tensor_path_hash,
tensor_data_row_id,
&decoded_tensor,
&tensor_stats,
&annotations,
Expand Down
Loading
Loading