From 2b97d32db53ffb773e10d585e0a184ab40666036 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Thu, 29 Sep 2022 22:50:31 +1000 Subject: [PATCH 1/3] renderBox: avoid unnecessary use of dlist `renderBox` uses dlist to grow the result list while doing a render traversal. An ordinary fmap'd list construction suffices and offers a very small but measurable performance boost. Furthermore this is the only place the *dlist* library was used so *brick* no longer depends on it. --- brick.cabal | 1 - src/Brick/Widgets/Core.hs | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/brick.cabal b/brick.cabal index d7f341a..d89293f 100644 --- a/brick.cabal +++ b/brick.cabal @@ -123,7 +123,6 @@ library bimap >= 0.5 && < 0.6, data-clist >= 0.1, directory >= 1.2.5.0, - dlist, exceptions >= 0.10.0, filepath, containers >= 0.5.7, diff --git a/src/Brick/Widgets/Core.hs b/src/Brick/Widgets/Core.hs index 68a1c74..2c16ace 100644 --- a/src/Brick/Widgets/Core.hs +++ b/src/Brick/Widgets/Core.hs @@ -126,7 +126,6 @@ import Control.Monad.State.Strict import Control.Monad.Reader import qualified Data.Foldable as F import qualified Data.Text as T -import qualified Data.DList as DL import qualified Data.Map as M import qualified Data.Set as S import qualified Data.IMap as I @@ -642,15 +641,15 @@ renderBox br ws = let availPrimary = c^.(contextPrimary br) availSecondary = c^.(contextSecondary br) - renderHis _ prev [] = return $ DL.toList prev - renderHis remainingPrimary prev ((i, prim):rest) = do + renderHis _ [] = pure [] + renderHis remainingPrimary ((i, prim):rest) = do result <- render $ limitPrimary br remainingPrimary $ limitSecondary br availSecondary $ cropToContext prim - renderHis (remainingPrimary - (result^.imageL.(to $ imagePrimary br))) - (DL.snoc prev (i, result)) rest + let newRem = remainingPrimary - (result^.imageL.(to $ imagePrimary br)) + ((i, result) :) <$> renderHis newRem rest - renderedHis <- renderHis availPrimary DL.empty his + renderedHis <- renderHis availPrimary his renderedLows <- case lows of [] -> return [] From 35874d5b820469afa5d760353ccbc79eb9528fc2 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Thu, 29 Sep 2022 23:11:34 +1000 Subject: [PATCH 2/3] renderBox: use strict state monad Using the strict state monad to calculate `renderedHis` brings another small performance increase. I find the StateT version a little easier to comprehend too, although it's a subjective matter. --- src/Brick/Widgets/Core.hs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Brick/Widgets/Core.hs b/src/Brick/Widgets/Core.hs index 2c16ace..63b43c7 100644 --- a/src/Brick/Widgets/Core.hs +++ b/src/Brick/Widgets/Core.hs @@ -641,15 +641,14 @@ renderBox br ws = let availPrimary = c^.(contextPrimary br) availSecondary = c^.(contextSecondary br) - renderHis _ [] = pure [] - renderHis remainingPrimary ((i, prim):rest) = do - result <- render $ limitPrimary br remainingPrimary - $ limitSecondary br availSecondary - $ cropToContext prim - let newRem = remainingPrimary - (result^.imageL.(to $ imagePrimary br)) - ((i, result) :) <$> renderHis newRem rest + renderHi prim = do + remainingPrimary <- get + result <- lift $ render $ limitPrimary br remainingPrimary + $ limitSecondary br availSecondary + $ cropToContext prim + result <$ put (remainingPrimary - (result^.imageL.(to $ imagePrimary br))) - renderedHis <- renderHis availPrimary his + renderedHis <- evalStateT (traverse (traverse renderHi) his) availPrimary renderedLows <- case lows of [] -> return [] From c3e1ac8bfdfb225090cd15155dbe684eb75434f6 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Fri, 30 Sep 2022 07:31:43 +1000 Subject: [PATCH 3/3] =?UTF-8?q?renderBox:=20avoid=20O(n=C2=B2)=20slowdowns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `renderBox` computes translations by summing the primary dimensions of widgets preceding the current widget. This has O(n²) complexity and causes poor performance with long lists of widgets. In one purebred scenario `renderBox` contributes ~30% of the CPU time of the program. Use a State computation to accumulate the total offset during the traversal. In the scenario described above, this change caused `renderBox` to drop to ~4% of the program CPU time. Also remove an occurence of `sum` that calculateed the remaining primary dimension space after rendering of greedy widgets. We already calculate this value during the greedy widget render traversal, so reuse it. --- src/Brick/Widgets/Core.hs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/Brick/Widgets/Core.hs b/src/Brick/Widgets/Core.hs index 63b43c7..67e3b40 100644 --- a/src/Brick/Widgets/Core.hs +++ b/src/Brick/Widgets/Core.hs @@ -125,6 +125,7 @@ import Lens.Micro.Mtl (use, (%=)) import Control.Monad.State.Strict import Control.Monad.Reader import qualified Data.Foldable as F +import Data.Traversable (for) import qualified Data.Text as T import qualified Data.Map as M import qualified Data.Set as S @@ -646,24 +647,22 @@ renderBox br ws = result <- lift $ render $ limitPrimary br remainingPrimary $ limitSecondary br availSecondary $ cropToContext prim - result <$ put (remainingPrimary - (result^.imageL.(to $ imagePrimary br))) + result <$ (put $! remainingPrimary - (result^.imageL.(to $ imagePrimary br))) - renderedHis <- evalStateT (traverse (traverse renderHi) his) availPrimary + (renderedHis, remainingPrimary) <- + runStateT (traverse (traverse renderHi) his) availPrimary renderedLows <- case lows of [] -> return [] ls -> do - let remainingPrimary = c^.(contextPrimary br) - - (sum $ (^._2.imageL.(to $ imagePrimary br)) <$> renderedHis) - primaryPerLow = remainingPrimary `div` length ls + let primaryPerLow = remainingPrimary `div` length ls rest = remainingPrimary - (primaryPerLow * length ls) - secondaryPerLow = c^.(contextSecondary br) primaries = replicate rest (primaryPerLow + 1) <> replicate (length ls - rest) primaryPerLow let renderLow ((i, prim), pri) = (i,) <$> (render $ limitPrimary br pri - $ limitSecondary br secondaryPerLow + $ limitSecondary br availSecondary $ cropToContext prim) if remainingPrimary > 0 then mapM renderLow (zip ls primaries) else return [] @@ -671,11 +670,10 @@ renderBox br ws = let rendered = sortBy (compare `DF.on` fst) $ renderedHis ++ renderedLows allResults = snd <$> rendered allImages = (^.imageL) <$> allResults - allPrimaries = imagePrimary br <$> allImages - allTranslatedResults = (flip map) (zip [0..] allResults) $ \(i, result) -> - let off = locationFromOffset br offPrimary - offPrimary = sum $ take i allPrimaries - in addResultOffset off result + allTranslatedResults = flip evalState 0 $ for allResults $ \result -> do + offPrimary <- get + put $ offPrimary + (result ^. imageL . to (imagePrimary br)) + pure $ addResultOffset (locationFromOffset br offPrimary) result -- Determine the secondary dimension value to pad to. In a -- vertical box we want all images to be the same width to -- avoid attribute over-runs or blank spaces with the wrong