perf: faster styler mixins #43

2026-02-22T17:27:30Z

abart27 commented

2026-02-22 17:27:30 +00:00

(Migrated from github.com)

Optimizes styler mixins so that they don't clone the entire styler when used.

Also adds styler mixin usage to the stress_test demo alongside a frametime counter.

Optimizes styler mixins so that they don't clone the entire styler when used. Also adds styler mixin usage to the `stress_test` demo alongside a frametime counter.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-02-22 17:31:18 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Optimizes per-control styler_mixin application by avoiding cloning the full styler params table each time a mixin is used, and adds demo instrumentation/examples to validate the improvement.

Changes:

Reworked ugui.internal.deep_merge to apply overrides in-place and return a rollback function.
Updated apply_styler_mixin to use the new in-place merge + rollback instead of cloning ugui.standard_styler.params.
Enhanced demos: added a styler_mixin example in the stress test and added a frametime overlay to the base demo.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
mupen-lua-ugui.lua	Implements in-place deep merge with rollback and uses it for styler mixins to avoid full-table clones.
demos/stress_test.lua	Adds a `styler_mixin` example to exercise the new path during the stress test.
demos/base.lua	Adds a simple rolling 1-second average frametime overlay for demo/perf observation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview Optimizes per-control `styler_mixin` application by avoiding cloning the full styler params table each time a mixin is used, and adds demo instrumentation/examples to validate the improvement. **Changes:** - Reworked `ugui.internal.deep_merge` to apply overrides in-place and return a rollback function. - Updated `apply_styler_mixin` to use the new in-place merge + rollback instead of cloning `ugui.standard_styler.params`. - Enhanced demos: added a `styler_mixin` example in the stress test and added a frametime overlay to the base demo. ### Reviewed changes Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments. | File | Description | | ---- | ----------- | | mupen-lua-ugui.lua | Implements in-place deep merge with rollback and uses it for styler mixins to avoid full-table clones. | | demos/stress_test.lua | Adds a `styler_mixin` example to exercise the new path during the stress test. | | demos/base.lua | Adds a simple rolling 1-second average frametime overlay for demo/perf observation. | --- 💡 <a href="/mupen64/ugui/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

mupen-lua-ugui.lua

					
				@ -274,35 +274,36 @@ ugui.internal = {

				        return res

Rollback closures capture the loop variable key by reference. After the pairs() loop advances, all stored rollback functions may end up writing to the last key, so rollback can corrupt b instead of restoring it. Capture key in a new local (e.g., local k = key) when creating the rollback op (or store {t2, key, prev} tuples and apply them in rollback).

                    local k = key
                    rollback_ops[#rollback_ops + 1] = function()
                        t2_ref[k] = prev

Rollback closures capture the loop variable `key` by reference. After the `pairs()` loop advances, all stored rollback functions may end up writing to the last `key`, so rollback can corrupt `b` instead of restoring it. Capture `key` in a new local (e.g., `local k = key`) when creating the rollback op (or store `{t2, key, prev}` tuples and apply them in rollback). ```suggestion local k = key rollback_ops[#rollback_ops + 1] = function() t2_ref[k] = prev ```

mupen-lua-ugui.lua

This change alters how styler_mixin is applied/reverted (in-place mutation + rollback) and is easy to regress (especially for nested tables / numeric keys). There don't appear to be any tests covering control.styler_mixin behavior; please add a unit test that asserts (1) params are overridden during draw and (2) restored afterward.


    ---Unit test for apply_styler_mixin / styler_mixin behavior.
    ---Asserts that params are overridden during draw and restored afterward.
    test_apply_styler_mixin = function()
        -- Ensure standard_styler and its params table exist.
        ugui.standard_styler = ugui.standard_styler or {}

        -- Original params with nested table and numeric key.
        ugui.standard_styler.params = {
            color = 'blue',
            nested = { width = 5 },
            [1] = 'original',
        }

        local control = {
            styler_mixin = {
                color = 'red',
                nested = { width = 10 },
                [1] = 'mixin',
            },
        }

        local function tables_equal(a, b)
            if a == b then
                return true
            end
            if type(a) ~= 'table' or type(b) ~= 'table' then
                return false
            end
            for k, v in pairs(a) do
                local bv = b[k]
                if type(v) == 'table' and type(bv) == 'table' then
                    if not tables_equal(v, bv) then
                        return false
                    end
                else
                    if v ~= bv then
                        return false
                    end
                end
            end
            for k, _ in pairs(b) do
                if a[k] == nil then
                    return false
                end
            end
            return true
        end

        -- Capture original params for later comparison.
        local original_params = {
            color = ugui.standard_styler.params.color,
            nested = { width = ugui.standard_styler.params.nested.width },
            [1] = ugui.standard_styler.params[1],
        }

        -- Apply the styler mixin.
        local rollback = ugui.internal.apply_styler_mixin(control)

        -- During "draw": params should reflect the mixin overrides.
        ugui.internal.assert(
            ugui.standard_styler.params.color == 'red',
            'apply_styler_mixin did not override color as expected'
        )
        ugui.internal.assert(
            ugui.standard_styler.params.nested.width == 10,
            'apply_styler_mixin did not override nested.width as expected'
        )
        ugui.internal.assert(
            ugui.standard_styler.params[1] == 'mixin',
            'apply_styler_mixin did not override numeric key as expected'
        )

        -- Roll back the mixin.
        rollback()

        -- After rollback: params should be restored to their original values.
        ugui.internal.assert(
            tables_equal(ugui.standard_styler.params, original_params),
            'apply_styler_mixin did not restore params after rollback'
        )
    end,

This change alters how `styler_mixin` is applied/reverted (in-place mutation + rollback) and is easy to regress (especially for nested tables / numeric keys). There don't appear to be any tests covering `control.styler_mixin` behavior; please add a unit test that asserts (1) params are overridden during draw and (2) restored afterward. ```suggestion ---Unit test for apply_styler_mixin / styler_mixin behavior. ---Asserts that params are overridden during draw and restored afterward. test_apply_styler_mixin = function() -- Ensure standard_styler and its params table exist. ugui.standard_styler = ugui.standard_styler or {} -- Original params with nested table and numeric key. ugui.standard_styler.params = { color = 'blue', nested = { width = 5 }, [1] = 'original', } local control = { styler_mixin = { color = 'red', nested = { width = 10 }, [1] = 'mixin', }, } local function tables_equal(a, b) if a == b then return true end if type(a) ~= 'table' or type(b) ~= 'table' then return false end for k, v in pairs(a) do local bv = b[k] if type(v) == 'table' and type(bv) == 'table' then if not tables_equal(v, bv) then return false end else if v ~= bv then return false end end end for k, _ in pairs(b) do if a[k] == nil then return false end end return true end -- Capture original params for later comparison. local original_params = { color = ugui.standard_styler.params.color, nested = { width = ugui.standard_styler.params.nested.width }, [1] = ugui.standard_styler.params[1], } -- Apply the styler mixin. local rollback = ugui.internal.apply_styler_mixin(control) -- During "draw": params should reflect the mixin overrides. ugui.internal.assert( ugui.standard_styler.params.color == 'red', 'apply_styler_mixin did not override color as expected' ) ugui.internal.assert( ugui.standard_styler.params.nested.width == 10, 'apply_styler_mixin did not override nested.width as expected' ) ugui.internal.assert( ugui.standard_styler.params[1] == 'mixin', 'apply_styler_mixin did not override numeric key as expected' ) -- Roll back the mixin. rollback() -- After rollback: params should be restored to their original values. ugui.internal.assert( tables_equal(ugui.standard_styler.params, original_params), 'apply_styler_mixin did not restore params after rollback' ) end, ```

Sign in to join this conversation.

No reviewers