Merge pull request #123 from ImJimmi/interpreter-performance

Improve performance of `jive::Interpreter`
ImJimmi · Nov 22, 2023 · b936734 · b936734
2 parents 67871d1 + d432605
commit b936734
Show file tree

Hide file tree

Showing 45 changed files with 868 additions and 193 deletions.
diff --git a/.github/workflows/test-runner.yml b/.github/workflows/test-runner.yml
@@ -37,7 +37,7 @@ jobs:
                   submodules: "recursive"
 
             - name: Configure CMake
-              run: cmake -B ${{github.workspace}}/build -G Xcode -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DJIVE_BUILD_TEST_RUNNER=ON -DJIVE_ENABLE_COVERAGE=ON
+              run: cmake -B ${{github.workspace}}/build -G Xcode -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DJIVE_BUILD_TEST_RUNNER=ON -DJIVE_ENABLE_COVERAGE=ON -DJIVE_ENABLE_SANITISERS=ON
 
             - name: Build
               run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}}

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -4,8 +4,9 @@ project(JIVE
 )
 configure_file(${CMAKE_SOURCE_DIR}/version.txt.in version.txt)
 
-option(JIVE_BUILD_TEST_RUNNER "Build JIVE's test runner?" OFF)
-option(JIVE_BUILD_DEMO_RUNNER "Build JIVE's demo runner?" OFF)
+include(cmake/jive_options.cmake)
+include(cmake/jive_code_coverage.cmake)
+include(cmake/jive_compiler_and_linker_options.cmake)
 
 if (JIVE_BUILD_TEST_RUNNER OR JIVE_BUILD_DEMO_RUNNER)
     add_subdirectory(runners/libraries)

diff --git a/cmake/jive_code_coverage.cmake b/cmake/jive_code_coverage.cmake
@@ -0,0 +1,16 @@
+include_guard()
+
+add_library(jive_code_coverage INTERFACE)
+add_library(jive::code_coverage ALIAS jive_code_coverage)
+
+if (APPLE AND JIVE_ENABLE_COVERAGE)
+    target_compile_options(jive_code_coverage
+    INTERFACE
+        --coverage
+    )
+
+    target_link_options(jive_code_coverage
+    INTERFACE
+        --coverage
+    )
+endif()
diff --git a/cmake/jive_compiler_and_linker_options.cmake b/cmake/jive_compiler_and_linker_options.cmake
@@ -0,0 +1,46 @@
+include_guard()
+
+add_library(jive_compiler_and_linker_options INTERFACE)
+add_library(jive::compiler_and_linker_options ALIAS jive_compiler_and_linker_options)
+
+target_compile_definitions(jive_compiler_and_linker_options
+INTERFACE
+    JUCE_DISABLE_JUCE_VERSION_PRINTING=1
+    JUCE_DISPLAY_SPLASH_SCREEN=0
+    JUCE_SILENCE_XCODE_15_LINKER_WARNING=1
+    JUCE_USE_CURL=0
+    JUCE_WEB_BROWSER=0
+)
+
+if (APPLE)
+    target_compile_options(jive_compiler_and_linker_options
+    INTERFACE
+        -Werror
+    )
+
+    target_link_options(jive_compiler_and_linker_options
+    INTERFACE
+        -Wl,-weak_reference_mismatches,weak
+    )
+
+    if (JIVE_ENABLE_SANITISERS)
+        target_compile_options(jive_compiler_and_linker_options
+        INTERFACE
+            -fsanitize=address
+            -fsanitize=undefined
+        )
+
+        target_link_options(jive_compiler_and_linker_options
+        INTERFACE
+            -fsanitize=address
+            -fsanitize=undefined
+        )
+    endif()
+endif()
+
+if (MSVC)
+    target_compile_options(jive_compiler_and_linker_options
+    INTERFACE
+        /WX
+    )
+endif()
diff --git a/cmake/jive_options.cmake b/cmake/jive_options.cmake
@@ -0,0 +1,6 @@
+include_guard()
+
+option(JIVE_BUILD_TEST_RUNNER "Build JIVE's test runner?" OFF)
+option(JIVE_BUILD_DEMO_RUNNER "Build JIVE's demo runner?" OFF)
+option(JIVE_ENABLE_COVERAGE "Generate coverage reports when running tests?" OFF)
+option(JIVE_ENABLE_SANITISERS "Enable ASan, LSan, UBSan?" OFF)
diff --git a/jive_components/containers/jive_DocumentWindow.cpp b/jive_components/containers/jive_DocumentWindow.cpp
@@ -3,15 +3,21 @@
 namespace jive
 {
     DocumentWindow::DocumentWindow()
-        : juce::DocumentWindow{ "",
-                                juce::LookAndFeel::getDefaultLookAndFeel()
-                                    .findColour(ResizableWindow::backgroundColourId),
-                                DocumentWindow::allButtons }
-        , onCloseButtonPressed{ []() {
-            juce::JUCEApplication::getInstance()->systemRequestedQuit();
-        } }
+        : juce::DocumentWindow{
+            "",
+            juce::LookAndFeel::getDefaultLookAndFeel()
+                .findColour(ResizableWindow::backgroundColourId),
+            DocumentWindow::allButtons,
+        }
+        , onCloseButtonPressed{
+            [] {
+                juce::JUCEApplication::getInstance()->systemRequestedQuit();
+            },
+        }
     {
+#if !JIVE_UNIT_TESTS
         setVisible(true);
+#endif
     }
 
     void DocumentWindow::closeButtonPressed()

diff --git a/jive_core/geometry/jive_BoxModel.cpp b/jive_core/geometry/jive_BoxModel.cpp
@@ -17,6 +17,7 @@ namespace jive
         , border{ state, "border-width" }
         , margin{ state, "margin" }
         , isValid{ state, "box-model-valid" }
+        , callbackLock{ state, "box-model-callback-lock" }
     {
         if (!width.exists())
             width.setAuto();
@@ -33,11 +34,17 @@ namespace jive
         }
 
         const auto onBoxModelChanged = [this]() {
+            if (callbackLock.get())
+                return;
+
             isValid = true;
             listeners.call(&Listener::boxModelChanged, *this);
             invalidateParent();
         };
         const auto recalculateSize = [this, onBoxModelChanged]() {
+            if (callbackLock.get())
+                return;
+
             const auto sizeBefore = componentSize.get();
             componentSize = juce::Rectangle{
                 calculateComponentWidth(),
@@ -63,6 +70,9 @@ namespace jive
         componentSize.onValueChange = onBoxModelChanged;
 
         isValid.onValueChange = [this]() {
+            if (callbackLock.get())
+                return;
+
             if (!isValid.get())
                 listeners.call(&Listener::boxModelInvalidated, *this);
         };
@@ -121,11 +131,13 @@ namespace jive
 
     juce::Rectangle<float> BoxModel::getContentBounds() const
     {
-        return padding
-            .get()
-            .subtractedFrom(border
+        const auto bounds = padding
                                 .get()
-                                .subtractedFrom(getOuterBounds()));
+                                .subtractedFrom(border
+                                                    .get()
+                                                    .subtractedFrom(getOuterBounds()));
+        return bounds.withSize(juce::jmax(0.0f, bounds.getWidth()),
+                               juce::jmax(0.0f, bounds.getHeight()));
     }
 
     juce::Rectangle<float> BoxModel::getMinimumBounds() const
@@ -183,13 +195,34 @@ namespace jive
 
     void BoxModel::invalidateParent()
     {
-        if (!state.getParent().isValid())
-            return;
+        if (auto parent = state.getParent();
+            parent.isValid())
+        {
+            Property<bool> parentIsValid{ parent, isValid.id };
+            parentIsValid = true;
+            parentIsValid = false;
+        }
+    }
+
+    BoxModel::ScopedCallbackLock::ScopedCallbackLock(BoxModel& boxModelToLock)
+        : boxModel{ boxModelToLock }
+    {
+        boxModel.lock();
+    }
+
+    BoxModel::ScopedCallbackLock::~ScopedCallbackLock()
+    {
+        boxModel.unlock();
+    }
 
-        BoxModel parent{ state.getParent() };
+    void BoxModel::lock()
+    {
+        callbackLock = true;
+    }
 
-        parent.isValid = true;
-        parent.isValid = false;
+    void BoxModel::unlock()
+    {
+        callbackLock.clear();
     }
 } // namespace jive
 

diff --git a/jive_core/geometry/jive_BoxModel.h b/jive_core/geometry/jive_BoxModel.h
@@ -13,6 +13,25 @@ namespace jive
             virtual void boxModelInvalidated(BoxModel&) {}
         };
 
+        /** Prevents any callbacks being invoked on the given BoxModel object
+            until the lock instance is destroyed.
+
+            Use with caution! This may cause things to break if the BoxModel
+            isn't able to properly respond to vital changes in its given
+            ValueTree. However, so long as you're sure you know what you're
+            doing, this can be very useful when setting several properties at
+            once and you don't want callbacks firing for each one.
+        */
+        class ScopedCallbackLock
+        {
+        public:
+            explicit ScopedCallbackLock(BoxModel& boxModelToLock);
+            ~ScopedCallbackLock();
+
+        private:
+            BoxModel& boxModel;
+        };
+
         explicit BoxModel(juce::ValueTree sourceState);
 
         float getWidth() const;
@@ -47,6 +66,9 @@ namespace jive
         juce::ValueTree state;
 
     private:
+        void lock();
+        void unlock();
+
         juce::Rectangle<float> getParentBounds() const;
         float calculateComponentWidth() const;
         float calculateComponentHeight() const;
@@ -65,6 +87,7 @@ namespace jive
         Property<juce::BorderSize<float>> border;
         Property<juce::BorderSize<float>> margin;
         Property<bool> isValid;
+        Property<bool> callbackLock;
 
         juce::ListenerList<Listener> listeners;
 

diff --git a/jive_core/jive_core.cpp b/jive_core/jive_core.cpp
@@ -1,5 +1,6 @@
 #include "jive_core.h"
 
+#include "logging/jive_ConsoleProgressBar.cpp"
 #include "logging/jive_ScopeIndentedLogger.cpp"
 #include "logging/jive_StringStreams.cpp"
 

diff --git a/jive_core/jive_core.h b/jive_core/jive_core.h
@@ -12,6 +12,7 @@ END_JUCE_MODULE_DECLARATION */
 
 #include <juce_gui_basics/juce_gui_basics.h>
 
+#include "logging/jive_ConsoleProgressBar.h"
 #include "logging/jive_ScopeIndentedLogger.h"
 #include "logging/jive_StringStreams.h"
 

diff --git a/jive_core/logging/jive_ConsoleProgressBar.cpp b/jive_core/logging/jive_ConsoleProgressBar.cpp
@@ -0,0 +1,40 @@
+#include "jive_ConsoleProgressBar.h"
+
+namespace jive
+{
+    static const juce::Array<juce::juce_wchar> blocks{
+        ' ',                                       //
+        *juce::CharPointer_UTF8{ "\xe2\x96\x8f" }, // ▏
+        *juce::CharPointer_UTF8{ "\xe2\x96\x8e" }, // ▎
+        *juce::CharPointer_UTF8{ "\xe2\x96\x8d" }, // ▍
+        *juce::CharPointer_UTF8{ "\xe2\x96\x8c" }, // ▌
+        *juce::CharPointer_UTF8{ "\xe2\x96\x8b" }, // ▋
+        *juce::CharPointer_UTF8{ "\xe2\x96\x8a" }, // ▊
+        *juce::CharPointer_UTF8{ "\xe2\x96\x89" }, // ▉
+        *juce::CharPointer_UTF8{ "\xe2\x96\x88" }, // █
+    };
+
+    [[nodiscard]] juce::String buildProgressBar(double progressNormalised, int maxStringLength)
+    {
+        jassert(progressNormalised >= 0.0 && progressNormalised <= 1.0);
+        progressNormalised = juce::jlimit(0.0, 1.0, progressNormalised);
+
+        static constexpr auto decimalPlaces = 2;
+        static constexpr auto hundredPercentAndWhitespaceWidth = decimalPlaces + 6; // 100.00%
+        const auto barLength = maxStringLength - hundredPercentAndWhitespaceWidth - 2;
+        const auto filledPortionExactWidth = barLength * progressNormalised;
+        const auto numFull = static_cast<int>(std::floor(filledPortionExactWidth));
+        const auto partialBlock = blocks[juce::roundToInt((filledPortionExactWidth - numFull) * (blocks.size() - 1))];
+        const auto bar = juce::String{}
+                             .paddedRight(blocks.getLast(), numFull)
+                             .paddedRight(partialBlock, juce::jmin(numFull + 1, barLength))
+                             .paddedRight(blocks.getFirst(), barLength);
+
+        static const juce::String cap{ juce::CharPointer_UTF8("\xe2\x94\x82") };
+        const auto percentage = (juce::String{ progressNormalised * 100, decimalPlaces }
+                                 + "%")
+                                    .paddedLeft(' ', hundredPercentAndWhitespaceWidth);
+
+        return cap + bar + cap + percentage;
+    }
+} // namespace jive
diff --git a/jive_core/logging/jive_ConsoleProgressBar.h b/jive_core/logging/jive_ConsoleProgressBar.h
@@ -0,0 +1,6 @@
+#pragma once
+
+namespace jive
+{
+    [[nodiscard]] juce::String buildProgressBar(double progressNormalised, int maxStringLength = 50);
+} // namespace jive
diff --git a/jive_layouts/layout/gui-items/content/jive_Image.cpp b/jive_layouts/layout/gui-items/content/jive_Image.cpp
@@ -12,6 +12,8 @@ namespace jive
         , idealHeight{ state, "ideal-height" }
         , boxModel{ toType<CommonGuiItem>()->boxModel }
     {
+        const BoxModel::ScopedCallbackLock boxModelLock{ jive::boxModel(*this) };
+
         if (!placement.exists())
             placement = juce::RectanglePlacement::centred;
 
@@ -475,8 +477,9 @@ class ImageTest : public juce::UnitTest
             jive::Interpreter interpreter;
             auto parent = interpreter.interpret(tree);
             auto& item = *parent->getChildren()[0];
-            expectEquals(jive::BoxModel{ item.state }.getWidth(), 80.0f);
-            expectEquals(jive::BoxModel{ item.state }.getHeight(), 40.0f);
+            const auto& boxModel = jive::boxModel(item);
+            expectEquals(boxModel.getWidth(), 80.0f);
+            expectEquals(boxModel.getHeight(), 40.0f);
         }
         {
             juce::ValueTree tree{
@@ -511,8 +514,9 @@ class ImageTest : public juce::UnitTest
             jive::Interpreter interpreter;
             auto parent = interpreter.interpret(tree);
             auto& item = *parent->getChildren()[0];
-            expectEquals(jive::BoxModel{ item.state }.getWidth(), 155.0f);
-            expectEquals(jive::BoxModel{ item.state }.getHeight(), 155.0f);
+            const auto& boxModel = jive::boxModel(item);
+            expectEquals(boxModel.getWidth(), 155.0f);
+            expectEquals(boxModel.getHeight(), 155.0f);
         }
     }