Layout-2020: Serialize access to stylo thread pool.

When rendering a page containing an iframe, layout 2020
creates parallel 'Layout' threads which share workers
in the stylo thread pool.

Because of the way the 'StyleSharingCache' is designed
using TLS for storage of the LRU cache, this leads to
a double borrow of the cache when both layout threads
run concurrently.

More details about the issue can be found here:
https://gist.github.com/mukilan/ed57eb61b83237a05fbf6360ec5e33b0

This PR is a workaround until we find a more elegant/optimal
design that also can work for gecko. The fix for now is
simply to not allow multiple layouts in parallel.

Signed-off-by: Mukilan Thiyagarajan <me@mukilan.in>
This commit is contained in:
Mukilan Thiyagarajan 2023-05-24 21:25:37 +05:30
parent 43ebf6c82c
commit 05239f879e
3 changed files with 17 additions and 19 deletions

View file

@ -1332,10 +1332,10 @@ impl LayoutThread {
data.stylesheets_changed, data.stylesheets_changed,
); );
let pool; let pool = STYLE_THREAD_POOL.lock().unwrap();
let thread_pool = pool.pool();
let (thread_pool, num_threads) = if self.parallel_flag { let (thread_pool, num_threads) = if self.parallel_flag {
pool = STYLE_THREAD_POOL.pool(); (thread_pool.as_ref(), pool.num_threads.unwrap_or(1))
(pool.as_ref(), STYLE_THREAD_POOL.num_threads.unwrap_or(1))
} else { } else {
(None, 1) (None, 1)
}; };
@ -1419,6 +1419,7 @@ impl LayoutThread {
Some(&document), Some(&document),
&mut rw_data, &mut rw_data,
&mut layout_context, &mut layout_context,
thread_pool,
); );
} }
@ -1626,6 +1627,7 @@ impl LayoutThread {
document: Option<&ServoLayoutDocument<LayoutData>>, document: Option<&ServoLayoutDocument<LayoutData>>,
rw_data: &mut LayoutThreadData, rw_data: &mut LayoutThreadData,
context: &mut LayoutContext, context: &mut LayoutContext,
thread_pool: Option<&rayon::ThreadPool>,
) { ) {
Self::cancel_animations_for_nodes_not_in_flow_tree( Self::cancel_animations_for_nodes_not_in_flow_tree(
&mut *(context.style_context.animations.sets.write()), &mut *(context.style_context.animations.sets.write()),
@ -1683,14 +1685,6 @@ impl LayoutThread {
|| { || {
let profiler_metadata = self.profiler_metadata(); let profiler_metadata = self.profiler_metadata();
let pool;
let thread_pool = if self.parallel_flag {
pool = STYLE_THREAD_POOL.pool();
pool.as_ref()
} else {
None
};
if let Some(pool) = thread_pool { if let Some(pool) = thread_pool {
// Parallel mode. // Parallel mode.
LayoutThread::solve_constraints_parallel( LayoutThread::solve_constraints_parallel(

View file

@ -517,6 +517,7 @@ impl LayoutThread {
animation_timeline_value: f64, animation_timeline_value: f64,
animations: &DocumentAnimationSet, animations: &DocumentAnimationSet,
stylesheets_changed: bool, stylesheets_changed: bool,
use_rayon: bool,
) -> LayoutContext<'a> { ) -> LayoutContext<'a> {
let traversal_flags = match stylesheets_changed { let traversal_flags = match stylesheets_changed {
true => TraversalFlags::ForCSSRuleChanges, true => TraversalFlags::ForCSSRuleChanges,
@ -541,7 +542,7 @@ impl LayoutThread {
font_cache_thread: Mutex::new(self.font_cache_thread.clone()), font_cache_thread: Mutex::new(self.font_cache_thread.clone()),
webrender_image_cache: self.webrender_image_cache.clone(), webrender_image_cache: self.webrender_image_cache.clone(),
pending_images: Mutex::new(vec![]), pending_images: Mutex::new(vec![]),
use_rayon: STYLE_THREAD_POOL.pool().is_some(), use_rayon,
} }
} }
@ -989,6 +990,10 @@ impl LayoutThread {
self.stylist.flush(&guards, Some(root_element), Some(&map)); self.stylist.flush(&guards, Some(root_element), Some(&map));
let rayon_pool = STYLE_THREAD_POOL.lock().unwrap();
let rayon_pool = rayon_pool.pool();
let rayon_pool = rayon_pool.as_ref();
// Create a layout context for use throughout the following passes. // Create a layout context for use throughout the following passes.
let mut layout_context = self.build_layout_context( let mut layout_context = self.build_layout_context(
guards.clone(), guards.clone(),
@ -997,6 +1002,7 @@ impl LayoutThread {
data.animation_timeline_value, data.animation_timeline_value,
&data.animations, &data.animations,
data.stylesheets_changed, data.stylesheets_changed,
rayon_pool.is_some(),
); );
let dirty_root = unsafe { let dirty_root = unsafe {
@ -1012,9 +1018,6 @@ impl LayoutThread {
RecalcStyle::pre_traverse(dirty_root, shared) RecalcStyle::pre_traverse(dirty_root, shared)
}; };
let rayon_pool = STYLE_THREAD_POOL.pool();
let rayon_pool = rayon_pool.as_ref();
if token.should_traverse() { if token.should_traverse() {
let dirty_root: ServoLayoutNode<DOMLayoutData> = let dirty_root: ServoLayoutNode<DOMLayoutData> =
driver::traverse_dom(&traversal, token, rayon_pool).as_node(); driver::traverse_dom(&traversal, token, rayon_pool).as_node();

View file

@ -16,6 +16,7 @@ use parking_lot::{RwLock, RwLockReadGuard};
use rayon; use rayon;
use std::env; use std::env;
use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Mutex;
/// Global style data /// Global style data
pub struct GlobalStyleData { pub struct GlobalStyleData {
@ -74,7 +75,7 @@ impl StyleThreadPool {
} }
{ {
// Drop the pool. // Drop the pool.
let _ = STYLE_THREAD_POOL.style_thread_pool.write().take(); let _ = STYLE_THREAD_POOL.lock().unwrap().style_thread_pool.write().take();
} }
// Spin until all our threads are done. This will usually be pretty // Spin until all our threads are done. This will usually be pretty
// fast, as on shutdown there should be basically no threads left // fast, as on shutdown there should be basically no threads left
@ -104,7 +105,7 @@ impl StyleThreadPool {
lazy_static! { lazy_static! {
/// Global thread pool /// Global thread pool
pub static ref STYLE_THREAD_POOL: StyleThreadPool = { pub static ref STYLE_THREAD_POOL: Mutex<StyleThreadPool> = {
let stylo_threads = env::var("STYLO_THREADS") let stylo_threads = env::var("STYLO_THREADS")
.map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS value")); .map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS value"));
let mut num_threads = match stylo_threads { let mut num_threads = match stylo_threads {
@ -157,14 +158,14 @@ lazy_static! {
workers.ok() workers.ok()
}; };
StyleThreadPool { Mutex::new(StyleThreadPool {
num_threads: if num_threads > 0 { num_threads: if num_threads > 0 {
Some(num_threads) Some(num_threads)
} else { } else {
None None
}, },
style_thread_pool: RwLock::new(pool), style_thread_pool: RwLock::new(pool),
} })
}; };
/// Global style data /// Global style data