Backed out changeset bd37586e34ed (bug 1903977) for causing WR bustages CLOSED TREE

gfx/webrender_bindings/src/moz2d_renderer.rs

@@ -483,7 +483,6 @@ struct Job {
     dirty_rect: BlobDirtyRect,
     visible_rect: DeviceIntRect,
     tile_size: TileSize,
-    output: MutableTileBuffer,
 }
 
 /// Rasterizes gecko blob images.
@@ -503,7 +502,6 @@ impl AsyncBlobImageRasterizer for Moz2dBlobRasterizer {
         &mut self,
         requests: &[BlobImageParams],
         low_priority: bool,
-        tile_pool: &mut BlobTilePool,
     ) -> Vec<(BlobImageRequest, BlobImageResult)> {
         // All we do here is spin up our workers to callback into gecko to replay the drawing commands.
         gecko_profiler_label!(Graphics, Rasterization);
@@ -521,8 +519,6 @@ impl AsyncBlobImageRasterizer for Moz2dBlobRasterizer {
                 let blob = Arc::clone(&command.data);
                 assert!(!params.descriptor.rect.is_empty());
 
-                let buf_size = (params.descriptor.rect.area() * params.descriptor.format.bytes_per_pixel()) as usize;
-
                 Job {
                     request: params.request,
                     descriptor: params.descriptor,
@@ -530,7 +526,6 @@ impl AsyncBlobImageRasterizer for Moz2dBlobRasterizer {
                     visible_rect: command.visible_rect,
                     dirty_rect: params.dirty_rect,
                     tile_size: command.tile_size,
-                    output: tile_pool.get_buffer(buf_size),
                 }
             })
             .collect();
@@ -548,7 +543,7 @@ impl AsyncBlobImageRasterizer for Moz2dBlobRasterizer {
             requests.len() > 4
         };
 
-        let result = if should_parallelize {
+        if should_parallelize {
             // Parallel version synchronously installs a job on the thread pool which will
             // try to do the work in parallel.
             // This thread is blocked until the thread pool is done doing the work.
@@ -562,9 +557,7 @@ impl AsyncBlobImageRasterizer for Moz2dBlobRasterizer {
             }
         } else {
             requests.into_iter().map(rasterize_blob).collect()
-        };
+        }
-
-        result
     }
 }
 
@@ -581,9 +574,12 @@ fn autoreleasepool<T, F: FnOnce() -> T>(f: F) -> T {
     }
 }
 
-fn rasterize_blob(mut job: Job) -> (BlobImageRequest, BlobImageResult) {
+fn rasterize_blob(job: Job) -> (BlobImageRequest, BlobImageResult) {
     gecko_profiler_label!(Graphics, Rasterization);
     let descriptor = job.descriptor;
+    let buf_size = (descriptor.rect.area() * descriptor.format.bytes_per_pixel()) as usize;
+
+    let mut output = vec![0u8; buf_size];
 
     let dirty_rect = match job.dirty_rect {
         DirtyRect::Partial(rect) => Some(rect),
@@ -591,8 +587,6 @@ fn rasterize_blob(mut job: Job) -> (BlobImageRequest, BlobImageResult) {
     };
     assert!(!descriptor.rect.is_empty());
 
-    let request = job.request;
-
     let result = autoreleasepool(|| {
         unsafe {
             if wr_moz2d_render_cb(
@@ -601,9 +595,9 @@ fn rasterize_blob(mut job: Job) -> (BlobImageRequest, BlobImageResult) {
                 &descriptor.rect,
                 &job.visible_rect,
                 job.tile_size,
-                &request.tile,
+                &job.request.tile,
                 dirty_rect.as_ref(),
-                MutByteSlice::new(job.output.as_mut_slice()),
+                MutByteSlice::new(output.as_mut_slice()),
             ) {
                 // We want the dirty rect local to the tile rather than the whole image.
                 // TODO(nical): move that up and avoid recomupting the tile bounds in the callback
@@ -613,7 +607,7 @@ fn rasterize_blob(mut job: Job) -> (BlobImageRequest, BlobImageResult) {
 
                 Ok(RasterizedBlobImage {
                     rasterized_rect,
-                    data: job.output.into_arc(),
+                    data: Arc::new(output),
                 })
             } else {
                 panic!("Moz2D replay problem");
@@ -621,7 +615,7 @@ fn rasterize_blob(mut job: Job) -> (BlobImageRequest, BlobImageResult) {
         }
     });
 
-    (request, result)
+    (job.request, result)
 }
 
 impl BlobImageHandler for Moz2dBlobImageHandler {

gfx/wr/webrender/src/renderer/init.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use api::{BlobImageHandler, ColorF, CrashAnnotator, DocumentId, IdNamespace};
+use api::{BlobImageHandler, ColorF, IdNamespace, DocumentId, CrashAnnotator};
 use api::{VoidPtrToSizeFn, FontRenderMode, ImageFormat};
 use api::{RenderNotifier, ImageBufferKind};
 use api::units::*;
@@ -617,8 +617,6 @@ pub fn create_webrender_instance(
         let lp_builder = LowPrioritySceneBuilderThread {
             rx: low_priority_scene_rx,
             tx: scene_tx.clone(),
-            // TODO: hard-coded tile size.
-            tile_pool: api::BlobTilePool::new(),
         };
 
         thread::Builder::new().name(lp_scene_thread_name.clone()).spawn(move || {

gfx/wr/webrender/src/scene_builder_thread.rs

@@ -40,11 +40,11 @@ use crate::util::drain_filter;
 use std::thread;
 use std::time::Duration;
 
-fn rasterize_blobs(txn: &mut TransactionMsg, is_low_priority: bool, tile_pool: &mut api::BlobTilePool) {
+fn rasterize_blobs(txn: &mut TransactionMsg, is_low_priority: bool) {
     profile_scope!("rasterize_blobs");
 
     if let Some(ref mut rasterizer) = txn.blob_rasterizer {
-        let mut rasterized_blobs = rasterizer.rasterize(&txn.blob_requests, is_low_priority, tile_pool);
+        let mut rasterized_blobs = rasterizer.rasterize(&txn.blob_requests, is_low_priority);
         // try using the existing allocation if our current list is empty
         if txn.rasterized_blobs.is_empty() {
             txn.rasterized_blobs = rasterized_blobs;
@@ -244,7 +244,6 @@ pub struct SceneBuilderThread {
     capture_config: Option<CaptureConfig>,
     debug_flags: DebugFlags,
     recycler: SceneRecycler,
-    tile_pool: api::BlobTilePool,
 }
 
 pub struct SceneBuilderThreadChannels {
@@ -291,8 +290,6 @@ impl SceneBuilderThread {
             capture_config: None,
             debug_flags: DebugFlags::default(),
             recycler: SceneRecycler::new(),
-            // TODO: tile size is hard-coded here.
-            tile_pool: api::BlobTilePool::new(),
         }
     }
 
@@ -334,7 +331,6 @@ impl SceneBuilderThread {
 
                     // Now that we off the critical path, do some memory bookkeeping.
                     self.recycler.recycle_built_scene();
-                    self.tile_pool.cleanup();
                 }
                 Ok(SceneBuilderRequest::AddDocument(document_id, initial_size)) => {
                     let old = self.documents.insert(document_id, Document::new(
@@ -645,7 +641,7 @@ impl SceneBuilderThread {
             profile.start_time(profiler::BLOB_RASTERIZATION_TIME);
 
             let is_low_priority = false;
-            rasterize_blobs(&mut txn, is_low_priority, &mut self.tile_pool);
+            rasterize_blobs(&mut txn, is_low_priority);
 
             profile.end_time(profiler::BLOB_RASTERIZATION_TIME);
             Telemetry::record_rasterize_blobs_time(Duration::from_micros((profile.get(profiler::BLOB_RASTERIZATION_TIME).unwrap() * 1000.00) as u64));
@@ -775,7 +771,6 @@ impl SceneBuilderThread {
 pub struct LowPrioritySceneBuilderThread {
     pub rx: Receiver<SceneBuilderRequest>,
     pub tx: Sender<SceneBuilderRequest>,
-    pub tile_pool: api::BlobTilePool,
 }
 
 impl LowPrioritySceneBuilderThread {
@@ -787,7 +782,6 @@ impl LowPrioritySceneBuilderThread {
                         .map(|txn| self.process_transaction(txn))
                         .collect();
                     self.tx.send(SceneBuilderRequest::Transactions(txns)).unwrap();
-                    self.tile_pool.cleanup();
                 }
                 Ok(SceneBuilderRequest::ShutDown(sync)) => {
                     self.tx.send(SceneBuilderRequest::ShutDown(sync)).unwrap();
@@ -806,7 +800,7 @@ impl LowPrioritySceneBuilderThread {
     fn process_transaction(&mut self, mut txn: Box<TransactionMsg>) -> Box<TransactionMsg> {
         let is_low_priority = true;
         txn.profile.start_time(profiler::BLOB_RASTERIZATION_TIME);
-        rasterize_blobs(&mut txn, is_low_priority, &mut self.tile_pool);
+        rasterize_blobs(&mut txn, is_low_priority);
         txn.profile.end_time(profiler::BLOB_RASTERIZATION_TIME);
         Telemetry::record_rasterize_blobs_time(Duration::from_micros((txn.profile.get(profiler::BLOB_RASTERIZATION_TIME).unwrap() * 1000.00) as u64));
         txn.blob_requests = Vec::new();

gfx/wr/webrender_api/src/image.rs

@@ -421,8 +421,7 @@ pub trait AsyncBlobImageRasterizer : Send {
     fn rasterize(
         &mut self,
         requests: &[BlobImageParams],
-        low_priority: bool,
+        low_priority: bool
-        tile_pool: &mut crate::BlobTilePool,
     ) -> Vec<(BlobImageRequest, BlobImageResult)>;
 }
 

gfx/wr/webrender_api/src/lib.rs

@@ -42,7 +42,6 @@ mod display_list;
 mod font;
 mod gradient_builder;
 mod image;
-mod tile_pool;
 pub mod units;
 
 pub use crate::color::*;
@@ -52,7 +51,6 @@ pub use crate::display_list::*;
 pub use crate::font::*;
 pub use crate::gradient_builder::*;
 pub use crate::image::*;
-pub use crate::tile_pool::*;
 
 use crate::units::*;
 use crate::channel::Receiver;

gfx/wr/webrender_api/src/tile_pool.rs

@@ -1,147 +0,0 @@
-use std::sync::Arc;
-
-const NUM_TILE_BUCKETS: usize = 6;
-
-/// A pool of blob tile buffers to mitigate the overhead of
-/// allocating and deallocating blob tiles.
-///
-/// The pool keeps a strong reference to each allocated buffers and
-/// reuses the ones with a strong count of 1.
-pub struct BlobTilePool {
-    largest_size_class: usize,
-    buckets: [Vec<Arc<Vec<u8>>>; NUM_TILE_BUCKETS],
-}
-
-impl BlobTilePool {
-    pub fn new() -> Self {
-        // The default max tile size is actually 256, using 512 here
-        // so that this still works when experimenting with larger
-        // tile sizes. If we ever make larger adjustments, the buckets
-        // should be changed accordingly.
-        let max_tile_size = 512;
-        BlobTilePool {
-            largest_size_class: max_tile_size * max_tile_size * 4,
-            buckets: [
-                Vec::with_capacity(32),
-                Vec::with_capacity(32),
-                Vec::with_capacity(32),
-                Vec::with_capacity(32),
-                Vec::with_capacity(32),
-                Vec::with_capacity(32),
-            ],
-        }
-    }
-
-    /// Get or allocate a tile buffer of the requested size.
-    ///
-    /// The returned buffer is zero-inizitalized.
-    /// The length of the returned buffer is equal to the requested size,
-    /// however the buffer may be allocated with a larger capacity to
-    /// confirm to the pool's corresponding bucket tile size.
-    pub fn get_buffer(&mut self, requested_size: usize) -> MutableTileBuffer {
-        assert!(requested_size <= self.largest_size_class);
-        
-        let (bucket_idx, cap) = self.bucket_and_size(requested_size);
-        let bucket = &mut self.buckets[bucket_idx];
-        let mut selected_idx = None;
-        for (buf_idx, buffer) in bucket.iter().enumerate() {
-            if Arc::strong_count(buffer) == 1 {
-                selected_idx = Some(buf_idx);
-                break;
-            }
-        }
-
-        let ptr;
-        let strong_ref;
-        if let Some(idx) = selected_idx {
-            {
-                // This works because we just ensured the pool has the only strong
-                // ref to the buffer.
-                let buffer = Arc::get_mut(&mut bucket[idx]).unwrap();
-                debug_assert!(buffer.capacity() >= requested_size);
-                // Ensure the length is equal to the requested size. It's not
-                // strictly necessay for the tile pool but the texture upload
-                // code relies on it.
-                unsafe { buffer.set_len(requested_size); }
-
-                // zero-initialize
-                buffer.fill(0);
-
-                ptr = buffer.as_mut_ptr();
-            }
-            strong_ref = Arc::clone(&bucket[idx]);
-        } else {
-            // Allocate a buffer with the adequate capacity for the requested
-            // size's bucket.
-            let mut buf = vec![0; cap];
-            // Force the length to be the requested size.
-            unsafe { buf.set_len(requested_size) };
-
-            ptr = buf.as_mut_ptr();
-            strong_ref = Arc::new(buf);
-            // Track the new buffer.
-            bucket.push(Arc::clone(&strong_ref));
-        };
-
-        MutableTileBuffer {
-            ptr,
-            strong_ref,
-        }
-    }
-
-    fn bucket_and_size(&self, size: usize) -> (usize, usize) {
-        let mut next_size_class = self.largest_size_class / 4;
-        let mut idx = 0;
-        while size < next_size_class && idx < NUM_TILE_BUCKETS - 1 {
-            next_size_class /= 4;
-            idx += 1;
-        }
-
-        (idx, next_size_class * 4)
-    }
-
-    /// Go over all allocated tile buffers. For each bucket, deallocate some buffers
-    /// until the number of unused buffer is more than half of the buffers for that
-    /// bucket.
-    ///
-    /// In practice, if called regularly, this gradually lets go of blob tiles when
-    /// they are not used.
-    pub fn cleanup(&mut self) {
-        for bucket in &mut self.buckets {
-            let threshold = bucket.len() / 2;
-            let mut num_available = 0;
-            bucket.retain(&mut |buffer: &Arc<Vec<u8>>| {
-                if Arc::strong_count(buffer) > 1 {
-                    return true;
-                }
-
-                num_available += 1;
-                num_available < threshold
-            });
-        }
-    }
-}
-
-
-// The role of tile buffer is to encapsulate an Arc to the underlying buffer
-// with a reference count of at most 2 and a way to view the buffer's content
-// as a mutable slice, even though the reference count may be more than 1.
-// The safety of this relies on the other strong reference being held by the
-// tile pool which never accesses the buffer's content, so the only reference
-// that can access it is the `TileBuffer` itself.
-pub struct MutableTileBuffer {
-    strong_ref: Arc<Vec<u8>>,
-    ptr: *mut u8,
-}
-
-impl MutableTileBuffer {
-    pub fn as_mut_slice(&mut self) -> &mut[u8] {
-        unsafe { std::slice::from_raw_parts_mut(self.ptr, self.strong_ref.len()) }
-    }
-
-    pub fn into_arc(self) -> Arc<Vec<u8>> {
-        self.strong_ref
-    }
-}
-
-unsafe impl Send for MutableTileBuffer {}