From 51a30623165fb592d5335b3b0359443f4344084e Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 11 Dec 2024 15:23:33 +0100 Subject: [PATCH 1/2] Remove some too frequent profile scopes (causing profiler slowdown for many-entities) (#8414) The usual whack-a-mole: profiler scopes themselves cause as more overhead than what they're profiling, causing puffing traces to be a lot less useful: recording of revy's `alien_cake_addict` Before (main): image After: image Puffing no longer warns but there's still quite some overhead: It makes the reported cpu time go up from ~52ms to ~57ms Well, time to call less methods - afaik a lot is caused by blueprint queries that we _know_ will turn up empty! (one of the next things I'm looking into) --- crates/store/re_query/src/latest_at.rs | 4 +--- crates/viewer/re_view/src/query.rs | 2 +- crates/viewer/re_view/src/results_ext.rs | 2 +- .../viewer/re_view_spatial/src/contexts/transform_context.rs | 2 +- crates/viewer/re_view_spatial/src/mesh_cache.rs | 2 -- crates/viewer/re_viewport_blueprint/src/view_contents.rs | 4 +--- 6 files changed, 5 insertions(+), 11 deletions(-) diff --git a/crates/store/re_query/src/latest_at.rs b/crates/store/re_query/src/latest_at.rs index e4f641665dd6..02a42649a4cb 100644 --- a/crates/store/re_query/src/latest_at.rs +++ b/crates/store/re_query/src/latest_at.rs @@ -48,7 +48,7 @@ impl QueryCache { entity_path: &EntityPath, component_descrs: impl IntoIterator>>, ) -> LatestAtResults { - re_tracing::profile_function!(entity_path.to_string()); + // This is called very frequently, don't put a profile scope here. let store = self.store.read(); @@ -87,8 +87,6 @@ impl QueryCache { // the data. let mut max_clear_index = (TimeInt::MIN, RowId::ZERO); { - re_tracing::profile_scope!("clears"); - let potential_clears = self.might_require_clearing.read(); let mut clear_entity_path = entity_path.clone(); diff --git a/crates/viewer/re_view/src/query.rs b/crates/viewer/re_view/src/query.rs index e554f4d07fef..98f3bcdd2ab5 100644 --- a/crates/viewer/re_view/src/query.rs +++ b/crates/viewer/re_view/src/query.rs @@ -85,7 +85,7 @@ pub fn latest_at_with_blueprint_resolved_data<'a>( component_names: impl IntoIterator, query_shadowed_defaults: bool, ) -> HybridLatestAtResults<'a> { - re_tracing::profile_function!(data_result.entity_path.to_string()); + // This is called very frequently, don't put a profile scope here. let mut component_set = component_names.into_iter().collect::>(); diff --git a/crates/viewer/re_view/src/results_ext.rs b/crates/viewer/re_view/src/results_ext.rs index 71eca73654c3..8e9300a4d508 100644 --- a/crates/viewer/re_view/src/results_ext.rs +++ b/crates/viewer/re_view/src/results_ext.rs @@ -135,7 +135,7 @@ pub enum HybridResults<'a> { impl HybridResults<'_> { pub fn query_result_hash(&self) -> Hash64 { - re_tracing::profile_function!(); + // This is called very frequently, don't put a profile scope here. // TODO(andreas): We should be able to do better than this and determine hashes for queries on the fly. match self { diff --git a/crates/viewer/re_view_spatial/src/contexts/transform_context.rs b/crates/viewer/re_view_spatial/src/contexts/transform_context.rs index 20122a8313ed..82a596ea58b6 100644 --- a/crates/viewer/re_view_spatial/src/contexts/transform_context.rs +++ b/crates/viewer/re_view_spatial/src/contexts/transform_context.rs @@ -734,7 +734,7 @@ fn transforms_at( pinhole_image_plane_distance: impl Fn(&EntityPath) -> f32, encountered_pinhole: &mut Option, ) -> Result { - re_tracing::profile_function!(); + // This is called very frequently, don't put a profile scope here. let potential_transform_components = TransformComponentTrackerStoreSubscriber::access(&entity_db.store_id(), |tracker| { diff --git a/crates/viewer/re_view_spatial/src/mesh_cache.rs b/crates/viewer/re_view_spatial/src/mesh_cache.rs index ff95fa76859b..9da0b806a3b5 100644 --- a/crates/viewer/re_view_spatial/src/mesh_cache.rs +++ b/crates/viewer/re_view_spatial/src/mesh_cache.rs @@ -56,8 +56,6 @@ impl MeshCache { mesh: AnyMesh<'_>, render_ctx: &RenderContext, ) -> Option> { - re_tracing::profile_function!(); - self.0 .entry(key.versioned_instance_path_hash.row_id) .or_default() diff --git a/crates/viewer/re_viewport_blueprint/src/view_contents.rs b/crates/viewer/re_viewport_blueprint/src/view_contents.rs index 720be6c9ccec..06fd584df7b8 100644 --- a/crates/viewer/re_viewport_blueprint/src/view_contents.rs +++ b/crates/viewer/re_viewport_blueprint/src/view_contents.rs @@ -393,8 +393,6 @@ impl DataQueryPropertyResolver<'_> { // Update visualizers from overrides. if !node.data_result.visualizers.is_empty() { - re_tracing::profile_scope!("Update visualizers from overrides"); - // If the user has overridden the visualizers, update which visualizers are used. if let Some(viz_override) = blueprint .latest_at_component::( @@ -524,7 +522,7 @@ impl DataQueryPropertyResolver<'_> { query_result: &mut DataQueryResult, view_states: &mut ViewStates, ) { - re_tracing::profile_function!(); + // This is called very frequently, don't put a profile scope here. if let Some(root) = query_result.tree.root_handle() { let recursive_property_overrides = Default::default(); From 388c8aae45f024a00d473fd90ba92043c7fd6e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Wed, 11 Dec 2024 15:32:24 +0100 Subject: [PATCH 2/2] Partial revert #8390: removal of `Area` in `zoom_pan_area` (#8416) ### Related * #8390 (partial revert) ### What We encountered some problems with this implementation. Discussion: https://rerunio.slack.com/archives/C04MTSM2U91/p1733921898485069 --- crates/viewer/re_ui/src/zoom_pan_area.rs | 67 +++++++++++++----------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/crates/viewer/re_ui/src/zoom_pan_area.rs b/crates/viewer/re_ui/src/zoom_pan_area.rs index 9dac80ce4130..46020aa15c34 100644 --- a/crates/viewer/re_ui/src/zoom_pan_area.rs +++ b/crates/viewer/re_ui/src/zoom_pan_area.rs @@ -5,7 +5,7 @@ //! * `view`-space: The space where the pan-and-zoom area is drawn. //! * `scene`-space: The space where the actual content is drawn. -use egui::{emath::TSTransform, Rect, Response, Ui, UiBuilder}; +use egui::{emath::TSTransform, Area, Rect, Response, Ui, UiKind}; /// Helper function to handle pan and zoom interactions on a response. fn register_pan_and_zoom(ui: &Ui, resp: &Response, ui_from_scene: &mut TSTransform) { @@ -58,42 +58,45 @@ pub fn fit_to_rect_in_scene(rect_in_ui: Rect, rect_in_scene: Rect) -> TSTransfor /// Provides a zoom-pan area for a given view. pub fn zoom_pan_area( - ui: &mut Ui, + ui: &Ui, view_bounds_in_ui: Rect, ui_from_scene: &mut TSTransform, draw_contents: impl FnOnce(&mut Ui), ) -> Response { - let zoom_pan_layer_id = egui::LayerId::new(ui.layer_id().order, ui.id().with("zoom_pan_area")); - - // Put the layer directly on-top of the main layer of the ui: - ui.ctx().set_sublayer(ui.layer_id(), zoom_pan_layer_id); - - let mut ui = ui.new_child( - UiBuilder::new() - .layer_id(zoom_pan_layer_id) - .max_rect(view_bounds_in_ui) - .sense(egui::Sense::click_and_drag()), - ); - - // Transform to the scene space: - let visible_rect_in_scene = ui_from_scene.inverse() * view_bounds_in_ui; - - // set proper clip-rect so we can interact with the background: - ui.set_clip_rect(visible_rect_in_scene); - - let pan_response = ui.response(); - - // Update the transform based on the interactions: - register_pan_and_zoom(&ui, &pan_response, ui_from_scene); - - // Update the clip-rect with the new transform, to avoid frame-delays - ui.set_clip_rect(ui_from_scene.inverse() * view_bounds_in_ui); - - // Add the actual contents to the area: - draw_contents(&mut ui); + let area_resp = Area::new(ui.id().with("zoom_pan_area")) + .constrain_to(view_bounds_in_ui) + .order(ui.layer_id().order) + .kind(UiKind::GenericArea) + .show(ui.ctx(), |ui| { + // Transform to the scene space: + let visible_rect_in_scene = ui_from_scene.inverse() * view_bounds_in_ui; + + // set proper clip-rect so we can interact with the background. + ui.set_clip_rect(visible_rect_in_scene); + + // A Ui for sensing drag-to-pan, scroll-to-zoom, etc + let mut drag_sense_ui = ui.new_child( + egui::UiBuilder::new() + .sense(egui::Sense::click_and_drag()) + .max_rect(visible_rect_in_scene), + ); + + drag_sense_ui.set_min_size(visible_rect_in_scene.size()); + let pan_response = drag_sense_ui.response(); + + // Update the transform based on the interactions: + register_pan_and_zoom(ui, &pan_response, ui_from_scene); + + // Update the clip-rect with the new transform, to avoid frame-delays + ui.set_clip_rect(ui_from_scene.inverse() * view_bounds_in_ui); + + // Add the actual contents to the area: + draw_contents(ui); + pan_response + }); ui.ctx() - .set_transform_layer(zoom_pan_layer_id, *ui_from_scene); + .set_transform_layer(area_resp.response.layer_id, *ui_from_scene); - pan_response + area_resp.inner }