From ebaeeedc6c3fa728e9d64f72a9fcec77e36fe691 Mon Sep 17 00:00:00 2001 From: Norisz Fay Date: Wed, 7 Aug 2024 10:37:22 +0300 Subject: [PATCH] Backed out changeset bd37586e34ed (bug 1903977) for causing WR bustages CLOSED TREE --- gfx/webrender_bindings/src/moz2d_renderer.rs | 26 ++-- gfx/wr/webrender/src/renderer/init.rs | 4 +- gfx/wr/webrender/src/scene_builder_thread.rs | 14 +- gfx/wr/webrender_api/src/image.rs | 3 +- gfx/wr/webrender_api/src/lib.rs | 2 - gfx/wr/webrender_api/src/tile_pool.rs | 147 ------------------- 6 files changed, 16 insertions(+), 180 deletions(-) delete mode 100644 gfx/wr/webrender_api/src/tile_pool.rs diff --git a/gfx/webrender_bindings/src/moz2d_renderer.rs b/gfx/webrender_bindings/src/moz2d_renderer.rs index 58b5bdf0b0df..c2eec0856c1f 100644 --- a/gfx/webrender_bindings/src/moz2d_renderer.rs +++ b/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: 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 { diff --git a/gfx/wr/webrender/src/renderer/init.rs b/gfx/wr/webrender/src/renderer/init.rs index a95a058b7c16..05fbb28b5890 100644 --- a/gfx/wr/webrender/src/renderer/init.rs +++ b/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 || { diff --git a/gfx/wr/webrender/src/scene_builder_thread.rs b/gfx/wr/webrender/src/scene_builder_thread.rs index e4c4de54849f..1aa1e778363e 100644 --- a/gfx/wr/webrender/src/scene_builder_thread.rs +++ b/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, 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, pub tx: Sender, - 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) -> Box { 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(); diff --git a/gfx/wr/webrender_api/src/image.rs b/gfx/wr/webrender_api/src/image.rs index d90e11e39e4a..903b1277823a 100644 --- a/gfx/wr/webrender_api/src/image.rs +++ b/gfx/wr/webrender_api/src/image.rs @@ -421,8 +421,7 @@ pub trait AsyncBlobImageRasterizer : Send { fn rasterize( &mut self, requests: &[BlobImageParams], - low_priority: bool, - tile_pool: &mut crate::BlobTilePool, + low_priority: bool ) -> Vec<(BlobImageRequest, BlobImageResult)>; } diff --git a/gfx/wr/webrender_api/src/lib.rs b/gfx/wr/webrender_api/src/lib.rs index 32019b52c482..8b2bf327ca4b 100644 --- a/gfx/wr/webrender_api/src/lib.rs +++ b/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; diff --git a/gfx/wr/webrender_api/src/tile_pool.rs b/gfx/wr/webrender_api/src/tile_pool.rs deleted file mode 100644 index 813281745027..000000000000 --- a/gfx/wr/webrender_api/src/tile_pool.rs +++ /dev/null @@ -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>>; 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>| { - 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>, - 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> { - self.strong_ref - } -} - -unsafe impl Send for MutableTileBuffer {}