UIManager: Optimize binary insert (#9600)

* Optimiz

frontend/ui/uimanager.lua

@@ -227,35 +227,24 @@ end
 
 -- Schedule an execution task; task queue is in ascending order
 function UIManager:schedule(sched_time, action, ...)
-    local p, s, e = 1, 1, #self._task_queue
-    if e ~= 0 then
-        -- Do a binary insert.
-        repeat
-            p = bit.rshift(e + s, 1) -- Not necessary to use (s + (e -s) / 2) here!
-            local p_time = self._task_queue[p].time
-            if sched_time > p_time then
-                if s == e then
-                    p = e + 1
-                    break
-                elseif s + 1 == e then
-                    s = e
-                else
-                    s = p
-                end
-            elseif sched_time < p_time then
-                if s == p then
-                    break
-                end
-                e = p
-            else
-                -- For fairness, it's better to make sure p+1 is strictly less than p.
-                -- Might want to revisit that in the future.
-                break
-            end
-        until e < s
+    local lo, hi = 1, #self._task_queue
+    -- Rightmost binary insertion
+    while lo <= hi do
+        -- NOTE: We should be (mostly) free from overflow here, thanks to LuaJIT's BitOp semantics.
+        --       For more fun details about this particular overflow,
+        --       c.f., https://ai.googleblog.com/2006/06/extra-extra-read-all-about-it-nearly.html
+        -- NOTE: For more fun reading about the binary search algo in general,
+        --       c.f., https://reprog.wordpress.com/2010/04/19/are-you-one-of-the-10-percent/
+        local mid = bit.rshift(lo + hi, 1)
+        local mid_time = self._task_queue[mid].time
+        if sched_time >= mid_time then
+            lo = mid + 1
+        else
+            hi = mid - 1
+        end
     end
 
-    table.insert(self._task_queue, p, {
+    table.insert(self._task_queue, lo, {
         time = sched_time,
         action = action,
         argc = select("#", ...),

spec/unit/uimanager_bench.lua

@@ -7,6 +7,19 @@ local time = require("ui/time")
 local NB_TESTS = 40000
 local noop = function() end
 
+local function check()
+    for i = 1, #UIManager._task_queue-1 do
+        -- test for wrongly inserted time
+        assert.is_true(UIManager._task_queue[i].time <= UIManager._task_queue[i+1].time,
+            "time wrongly sorted")
+        if UIManager._task_queue[i].time == UIManager._task_queue[i+1].time then
+            -- for same time, test if later inserted action is after a former action
+            assert.is_true(UIManager._task_queue[i].action <= UIManager._task_queue[i+1].action,
+                "ragnarock")
+        end
+    end
+end
+
 describe("UIManager checkTasks benchmark", function()
     local now = time.now()
     local wait_until -- luacheck: no unused
@@ -25,7 +38,7 @@ describe("UIManager checkTasks benchmark", function()
     end
 end)
 
-describe("UIManager schedule benchmark", function()
+describe("UIManager schedule simple benchmark", function()
     local now = time.now()
     UIManager:quit()
     UIManager._task_queue = {}
@@ -35,6 +48,102 @@ describe("UIManager schedule benchmark", function()
     end
 end)
 
+describe("UIManager schedule more sophiticated benchmark", function()
+    -- This BM is doing schedulings like the are done in real usage
+    -- with autosuspend, autodim, autowarmth and friends.
+    local now = time.now()
+    UIManager:quit()
+
+    local function standby_dummy() end
+    local function autowarmth_dummy() end
+    local function dimmer_dummy() end
+
+    local function someTaps()
+        for j = 1,10 do
+            -- insert some random times for entering standby
+            UIManager:schedule(now + time.s(j), standby_dummy) -- standby
+            UIManager:unschedule(standby_dummy)
+        end
+    end
+
+    for i=1, NB_TESTS do
+        UIManager._task_queue = {}
+        UIManager:schedule(now + time.s(24*60*60), noop) -- shutdown
+        UIManager:schedule(now + time.s(15*60*60), noop) -- sleep
+        UIManager:schedule(now + time.s(55), noop) -- footer refresh
+        UIManager:schedule(now + time.s(130), noop) -- something
+        UIManager:schedule(now + time.s(10), noop) -- something else
+
+        for j = 1,5 do
+            someTaps()
+            UIManager:schedule(now + time.s(15*60), autowarmth_dummy) -- autowarmth
+            UIManager:schedule(now + time.s(180), dimmer_dummy) -- dimmer
+
+            now = now + 30
+            UIManager:unschedule(dimmer_dummy)
+            UIManager:unschedule(autowarmth_dummy) -- remove autowarmth
+        end
+    end
+end)
+
+describe("UIManager schedule massive collision tests", function()
+    print("Doing massive collision tests ......... this takes a lot of time")
+    UIManager:quit()
+
+    for i = 1, 6 do
+        -- simple test (1000/10 collisions)
+        UIManager._task_queue = {}
+        for j = 1, 10 do
+            UIManager:schedule(math.random(10), j)
+            -- check() -- enabling this takes really long O(n^2)
+        end
+        check()
+
+        -- armageddon test (10000 collisions)
+        UIManager._task_queue = {}
+        for j = 1, 1e5 do
+            UIManager:schedule(math.random(100), j)
+            -- check() -- enabling this takes really long O(n^2)
+        end
+        check()
+    end
+end)
+
+
+describe("UIManager schedule massive rediculous tests", function()
+    print("Doing massive rediculous collision tests ......... this takes really a lot time")
+    UIManager:quit()
+
+    for i = 1, 6 do
+        -- simple test (1000 collisions)
+        UIManager._task_queue = {}
+        local offs = 0
+        for j = 1, 1e3 do
+            UIManager:schedule(math.random(10), j + offs)
+            offs = offs + 1
+            -- check() -- enabling this takes really long O(n^2)
+        end
+        check()
+
+        -- simple (unknown number of collisions and times)
+        for j = 1, 1e4 do
+            UIManager:schedule(math.random(), j + offs)
+            offs = offs + 1
+            -- check() -- enabling this takes really long O(n^2)
+        end
+        check()
+
+        -- armageddon test (100 collisions)
+        for j = 1, 1e5 do
+            UIManager:schedule(math.random(math.random(100)), j + offs)
+            offs = offs + 1
+            -- check() -- enabling this takes really long O(n^2)
+        end
+        check()
+    end
+
+end)
+
 describe("UIManager unschedule benchmark", function()
     local now = time.now()
     UIManager:quit()

spec/unit/uimanager_spec.lua

@@ -113,6 +113,52 @@ describe("UIManager spec", function()
         assert.are.same('quux', UIManager._task_queue[5].action)
     end)
 
+    it("should insert new tasks with same times after existing tasks", function()
+        now = time.now()
+        UIManager:quit()
+        UIManager._task_queue = {}
+
+        -- insert task "5s" between "now" and "10s"
+        UIManager:schedule(now, "now");
+        assert.are.same("now", UIManager._task_queue[1].action)
+        UIManager:schedule(now + time.s(10), "10s");
+        assert.are.same("10s", UIManager._task_queue[2].action)
+        UIManager:schedule(now + time.s(5), "5s");
+        assert.are.same("5s", UIManager._task_queue[2].action)
+
+        -- insert task at the end after "10s"
+        UIManager:scheduleIn(10, 'foo') -- is a bit later than "10s", as time.now() is used internally
+        assert.are.same('foo', UIManager._task_queue[4].action)
+
+        -- insert task at the second last position after "10s"
+        UIManager:schedule(now + time.s(10), 'bar')
+        assert.are.same('bar', UIManager._task_queue[4].action)
+
+        -- insert task at the second last position after "bar"
+        UIManager:schedule(now + time.s(10), 'baz')
+        assert.are.same('baz', UIManager._task_queue[5].action)
+
+        -- insert task after "5s"
+        UIManager:schedule(now + time.s(5), 'nix')
+        assert.are.same('nix', UIManager._task_queue[3].action)
+        -- "barba" is later than "nix" anyway
+        UIManager:scheduleIn(5, 'barba') -- is a bit later than "5s", as time.now() is used internally
+        assert.are.same('barba', UIManager._task_queue[4].action)
+
+        -- "papa" is shortly after "now"
+        UIManager:nextTick('papa') -- is a bit later than "now"
+        assert.are.same('papa', UIManager._task_queue[2].action)
+
+        -- "mama is shedule now and inserted after "now"
+        UIManager:schedule(now, 'mama')
+        assert.are.same('mama', UIManager._task_queue[2].action)
+
+        -- "letta" is shortly after "papa"
+        UIManager:tickAfterNext('letta')
+        assert.are.same("function", type(UIManager._task_queue[4].action))
+
+    end)
+
     it("should unschedule all the tasks with the same action", function()
         now = time.now()
         UIManager:quit()