From 4132b605e444cde1ee5d1ddd9dd9160ca376bd21 Mon Sep 17 00:00:00 2001
From: James Liu <contact@jamessliu.com>
Date: Tue, 21 Jun 2022 18:10:27 +0000
Subject: [PATCH] Change check_visibility to use thread-local queues instead of
 a channel (#4663)

Further speed up visibility checking by removing the main sources of contention for the system.

 - ~~Make `ComputedVisibility` a resource wrapping a `FixedBitset`.~~
 - ~~Remove `ComputedVisibility` as a component.~~

~~This adds a one-bit overhead to every entity in the app world. For a game with 100,000 entities, this is 12.5KB of memory. This is still small enough to fit entirely in most L1 caches. Also removes the need for a per-Entity change detection tick. This reduces the memory footprint of ComputedVisibility 72x.~~

~~The decreased memory usage and less fragmented memory locality should provide significant performance benefits.~~

~~Clearing visible entities should be significantly faster than before:~~

 - ~~Setting one `u32` to 0 clears 32 entities per cycle.~~
 - ~~No archetype fragmentation to contend with.~~
 - ~~Change detection is applied to the resource, so there is no per-Entity update tick requirement.~~

~~The side benefit of this design is that it removes one more "computed component" from userspace.  Though accessing the values within it are now less ergonomic.~~

This PR changes `crossbeam_channel` in `check_visibility` to use a `Local<ThreadLocal<Cell<Vec<Entity>>>` to mark down visible entities instead.

Co-Authored-By: TheRawMeatball <therawmeatball@gmail.com>
Co-Authored-By: Aevyrie <aevyrie@gmail.com>
---
 crates/bevy_render/Cargo.toml                 |  2 +-
 crates/bevy_render/src/view/visibility/mod.rs | 45 ++++++++++++-------
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/crates/bevy_render/Cargo.toml b/crates/bevy_render/Cargo.toml
index fc9796e83f83f..879dbbbf9af9d 100644
--- a/crates/bevy_render/Cargo.toml
+++ b/crates/bevy_render/Cargo.toml
@@ -54,9 +54,9 @@ bitflags = "1.2.1"
 smallvec = { version = "1.6", features = ["union", "const_generics"] }
 once_cell = "1.4.1" # TODO: replace once_cell with std equivalent if/when this lands: https://github.com/rust-lang/rfcs/pull/2788
 downcast-rs = "1.2.0"
+thread_local = "1.1"
 thiserror = "1.0"
 futures-lite = "1.4.0"
-crossbeam-channel = "0.5.0"
 anyhow = "1.0"
 hex = "0.4.2"
 hexasphere = "7.0.0"
diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs
index 4e966af66ccb2..c1bffec3f0c2c 100644
--- a/crates/bevy_render/src/view/visibility/mod.rs
+++ b/crates/bevy_render/src/view/visibility/mod.rs
@@ -10,6 +10,8 @@ use bevy_reflect::std_traits::ReflectDefault;
 use bevy_reflect::Reflect;
 use bevy_transform::components::GlobalTransform;
 use bevy_transform::TransformSystem;
+use std::cell::Cell;
+use thread_local::ThreadLocal;
 
 use crate::{
     camera::{Camera, CameraProjection, OrthographicProjection, PerspectiveProjection, Projection},
@@ -148,22 +150,31 @@ pub fn update_frusta<T: Component + CameraProjection + Send + Sync + 'static>(
 }
 
 pub fn check_visibility(
+    mut thread_queues: Local<ThreadLocal<Cell<Vec<Entity>>>>,
     mut view_query: Query<(&mut VisibleEntities, &Frustum, Option<&RenderLayers>), With<Camera>>,
-    mut visible_entity_query: Query<(
-        Entity,
-        &Visibility,
-        &mut ComputedVisibility,
-        Option<&RenderLayers>,
-        Option<&Aabb>,
-        Option<&NoFrustumCulling>,
-        Option<&GlobalTransform>,
+    mut visible_entity_query: ParamSet<(
+        Query<&mut ComputedVisibility>,
+        Query<(
+            Entity,
+            &Visibility,
+            &mut ComputedVisibility,
+            Option<&RenderLayers>,
+            Option<&Aabb>,
+            Option<&NoFrustumCulling>,
+            Option<&GlobalTransform>,
+        )>,
     )>,
 ) {
+    // Reset the computed visibility to false
+    for mut computed_visibility in visible_entity_query.p0().iter_mut() {
+        computed_visibility.is_visible = false;
+    }
+
     for (mut visible_entities, frustum, maybe_view_mask) in view_query.iter_mut() {
         let view_mask = maybe_view_mask.copied().unwrap_or_default();
-        let (visible_entity_sender, visible_entity_receiver) = crossbeam_channel::unbounded();
 
-        visible_entity_query.par_iter().for_each_mut(
+        visible_entities.entities.clear();
+        visible_entity_query.p1().par_iter().for_each_mut(
             |(
                 entity,
                 visibility,
@@ -173,12 +184,10 @@ pub fn check_visibility(
                 maybe_no_frustum_culling,
                 maybe_transform,
             )| {
-                // Reset visibility
-                computed_visibility.is_visible = false;
-
                 if !visibility.is_visible {
                     return;
                 }
+
                 let entity_mask = maybe_entity_mask.copied().unwrap_or_default();
                 if !view_mask.intersects(&entity_mask) {
                     return;
@@ -204,9 +213,15 @@ pub fn check_visibility(
                 }
 
                 computed_visibility.is_visible = true;
-                visible_entity_sender.send(entity).ok();
+                let cell = thread_queues.get_or_default();
+                let mut queue = cell.take();
+                queue.push(entity);
+                cell.set(queue);
             },
         );
-        visible_entities.entities = visible_entity_receiver.try_iter().collect();
+
+        for cell in thread_queues.iter_mut() {
+            visible_entities.entities.append(cell.get_mut());
+        }
     }
 }