fix: remove unnecessary synchronization in layout cache (#1245)

Layout::init_cache no longer returns bool and takes a NonZeroUsize instead of usize

The cache is a thread-local, so doesn't make much sense to require
synchronized initialization.

BREAKING-CHANGES.md

@@ -10,6 +10,8 @@ GitHub with a [breaking change] label.
 
 This is a quick summary of the sections below:
 
+- [v0.28.0](#v0280)
+  - `Layout::init_cache` no longer returns bool and takes a `NonZeroUsize` instead of `usize`
 - [v0.27.0](#v0270)
   - List no clamps the selected index to list
   - Prelude items added / removed

examples/constraints.rs

@@ -86,9 +86,6 @@ fn main() -> Result<()> {
     init_error_hooks()?;
     let terminal = init_terminal()?;
 
-    // increase the cache size to avoid flickering for indeterminate layouts
-    Layout::init_cache(100);
-
     App::default().run(terminal)?;
 
     restore_terminal()?;

examples/flex.rs

@@ -13,7 +13,10 @@
 //! [examples]: https://github.com/ratatui-org/ratatui/blob/main/examples
 //! [examples readme]: https://github.com/ratatui-org/ratatui/blob/main/examples/README.md
 
-use std::io::{self, stdout};
+use std::{
+    io::{self, stdout},
+    num::NonZeroUsize,
+};
 
 use color_eyre::{config::HookBuilder, Result};
 use ratatui::{
@@ -156,7 +159,9 @@ enum SelectedTab {
 
 fn main() -> Result<()> {
     // assuming the user changes spacing about a 100 times or so
-    Layout::init_cache(EXAMPLE_DATA.len() * SelectedTab::iter().len() * 100);
+    Layout::init_cache(
+        NonZeroUsize::new(EXAMPLE_DATA.len() * SelectedTab::iter().len() * 100).unwrap(),
+    );
     init_error_hooks()?;
     let terminal = init_terminal()?;
     App::default().run(terminal)?;

src/layout/layout.rs

@@ -1,4 +1,4 @@
-use std::{cell::RefCell, collections::HashMap, iter, num::NonZeroUsize, rc::Rc, sync::OnceLock};
+use std::{cell::RefCell, collections::HashMap, iter, num::NonZeroUsize, rc::Rc};
 
 use cassowary::{
     strength::REQUIRED,
@@ -37,7 +37,9 @@ type Cache = LruCache<(Rect, Layout), (Segments, Spacers)>;
 const FLOAT_PRECISION_MULTIPLIER: f64 = 100.0;
 
 thread_local! {
-    static LAYOUT_CACHE: OnceLock<RefCell<Cache>> = const { OnceLock::new() };
+    static LAYOUT_CACHE: RefCell<Cache> = RefCell::new(Cache::new(
+        NonZeroUsize::new(Layout::DEFAULT_CACHE_SIZE).unwrap(),
+    ));
 }
 
 /// A layout is a set of constraints that can be applied to a given area to split it into smaller
@@ -209,22 +211,9 @@ impl Layout {
     /// that subsequent calls with the same parameters are faster. The cache is a `LruCache`, and
     /// grows until `cache_size` is reached.
     ///
-    /// Returns true if the cell's value was set by this call.
-    /// Returns false if the cell's value was not set by this call, this means that another thread
-    /// has set this value or that the cache size is already initialized.
-    ///
-    /// Note that a custom cache size will be set only if this function:
-    /// * is called before [`Layout::split()`] otherwise, the cache size is
-    ///   [`Self::DEFAULT_CACHE_SIZE`].
-    /// * is called for the first time, subsequent calls do not modify the cache size.
-    pub fn init_cache(cache_size: usize) -> bool {
-        LAYOUT_CACHE
-            .with(|c| {
-                c.set(RefCell::new(LruCache::new(
-                    NonZeroUsize::new(cache_size).unwrap(),
-                )))
-            })
-            .is_ok()
+    /// By default, the cache size is [`Self::DEFAULT_CACHE_SIZE`].
+    pub fn init_cache(cache_size: NonZeroUsize) {
+        LAYOUT_CACHE.with_borrow_mut(|c| c.resize(cache_size));
     }
 
     /// Set the direction of the layout.
@@ -571,17 +560,10 @@ impl Layout {
     /// );
     /// ```
     pub fn split_with_spacers(&self, area: Rect) -> (Segments, Spacers) {
-        LAYOUT_CACHE.with(|c| {
-            c.get_or_init(|| {
-                RefCell::new(LruCache::new(
-                    NonZeroUsize::new(Self::DEFAULT_CACHE_SIZE).unwrap(),
-                ))
-            })
-            .borrow_mut()
-            .get_or_insert((area, self.clone()), || {
-                self.try_split(area).expect("failed to split")
-            })
-            .clone()
+        LAYOUT_CACHE.with_borrow_mut(|c| {
+            let key = (area, self.clone());
+            c.get_or_insert(key, || self.try_split(area).expect("failed to split"))
+                .clone()
         })
     }
 
@@ -1099,37 +1081,14 @@ mod tests {
     }
 
     #[test]
-    fn custom_cache_size() {
-        assert!(Layout::init_cache(10));
-        assert!(!Layout::init_cache(15));
-        LAYOUT_CACHE.with(|c| {
-            assert_eq!(c.get().unwrap().borrow().cap().get(), 10);
+    fn cache_size() {
+        LAYOUT_CACHE.with_borrow(|c| {
+            assert_eq!(c.cap().get(), Layout::DEFAULT_CACHE_SIZE);
         });
-    }
 
-    #[test]
-    fn default_cache_size() {
-        let target = Rect {
-            x: 2,
-            y: 2,
-            width: 10,
-            height: 10,
-        };
-
-        Layout::default()
-            .direction(Direction::Vertical)
-            .constraints([
-                Constraint::Percentage(10),
-                Constraint::Max(5),
-                Constraint::Min(1),
-            ])
-            .split(target);
-        assert!(!Layout::init_cache(15));
-        LAYOUT_CACHE.with(|c| {
-            assert_eq!(
-                c.get().unwrap().borrow().cap().get(),
-                Layout::DEFAULT_CACHE_SIZE
-            );
+        Layout::init_cache(NonZeroUsize::new(10).unwrap());
+        LAYOUT_CACHE.with_borrow(|c| {
+            assert_eq!(c.cap().get(), 10);
         });
     }