Make Rednet deduplication more efficient (#925)

src/main/resources/data/computercraft/lua/rom/apis/rednet.lua

@@ -30,8 +30,8 @@ CHANNEL_REPEAT = 65533
 MAX_ID_CHANNELS = 65500
 
 local tReceivedMessages = {}
-local tReceivedMessageTimeouts = {}
 local tHostnames = {}
+local nClearTimer
 
 local function id_as_channel(id)
     return (id or os.getComputerID()) % MAX_ID_CHANNELS
@@ -138,8 +138,8 @@ function send(nRecipient, message, sProtocol)
     -- We could do other things to guarantee uniqueness, but we really don't need to
     -- Store it to ensure we don't get our own messages back
     local nMessageID = math.random(1, 2147483647)
-    tReceivedMessages[nMessageID] = true
+    tReceivedMessages[nMessageID] = os.clock() + 9.5
-    tReceivedMessageTimeouts[os.startTimer(30)] = nMessageID
+    if not nClearTimer then nClearTimer = os.startTimer(10) end
 
     -- Create the message
     local nReplyChannel = id_as_channel()
@@ -408,8 +408,8 @@ function run()
                     and tMessage.nMessageID == tMessage.nMessageID and not tReceivedMessages[tMessage.nMessageID]
                     and ((tMessage.nRecipient and tMessage.nRecipient == os.getComputerID()) or nChannel == CHANNEL_BROADCAST)
                 then
-                    tReceivedMessages[tMessage.nMessageID] = true
+                    tReceivedMessages[tMessage.nMessageID] = os.clock() + 9.5
-                    tReceivedMessageTimeouts[os.startTimer(30)] = tMessage.nMessageID
+                    if not nClearTimer then nClearTimer = os.startTimer(10) end
                     os.queueEvent("rednet_message", tMessage.nSender or nReplyChannel, tMessage.message, tMessage.sProtocol)
                 end
             end
@@ -428,14 +428,15 @@ function run()
                 end
             end
 
-        elseif sEvent == "timer" then
+        elseif sEvent == "timer" and p1 == nClearTimer then
             -- Got a timer event, use it to clear the event queue
-            local nTimer = p1
+            nClearTimer = nil
-            local nMessage = tReceivedMessageTimeouts[nTimer]
+            local nNow, bHasMore = os.clock(), nil
-            if nMessage then
+            for nMessageID, nDeadline in pairs(tReceivedMessages) do
-                tReceivedMessageTimeouts[nTimer] = nil
+                if nDeadline <= nNow then tReceivedMessages[nMessageID] = nil
-                tReceivedMessages[nMessage] = nil
+                else bHasMore = true end
             end
+            nClearTimer = bHasMore and os.startTimer(10)
         end
     end
 end

src/test/resources/test-rom/spec/apis/rednet_spec.lua

@@ -86,21 +86,22 @@ describe("The rednet library", function()
 
     describe("on fake computers", function()
         local fake_computer = require "support.fake_computer"
+        local debugx = require "support.debug_ext"
 
         local function computer_with_rednet(id, fn, options)
-            local computer = fake_computer.make_computer(id, function(_ENV)
+            local computer = fake_computer.make_computer(id, function(env)
-                local fns = { _ENV.rednet.run }
+                local fns = { env.rednet.run }
                 if options and options.rep then
-                    fns[#fns + 1] = function() _ENV.dofile("rom/programs/rednet/repeat.lua") end
+                    fns[#fns + 1] = function() env.dofile("rom/programs/rednet/repeat.lua") end
                 end
 
                 if fn then
                     fns[#fns + 1] = function()
                         if options and options.open then
-                            _ENV.rednet.open("back")
+                            env.rednet.open("back")
-                            _ENV.os.queueEvent("x") _ENV.os.pullEvent("x")
+                            env.os.queueEvent("x") env.os.pullEvent("x")
                         end
-                        return fn(_ENV.rednet, _ENV)
+                        return fn(env.rednet, env)
                     end
                 end
 
@@ -140,7 +141,7 @@ describe("The rednet library", function()
         end)
 
         it("sends and receives rednet messages", function()
-            local computer_1, modem_1 = computer_with_rednet(1, function(rednet, _ENV)
+            local computer_1, modem_1 = computer_with_rednet(1, function(rednet)
                 rednet.send(2, "Hello")
             end, { open = true })
             local computer_2, modem_2 = computer_with_rednet(2, function(rednet)
@@ -154,7 +155,7 @@ describe("The rednet library", function()
         end)
 
         it("repeats messages between computers", function()
-            local computer_1, modem_1 = computer_with_rednet(1, function(rednet, _ENV)
+            local computer_1, modem_1 = computer_with_rednet(1, function(rednet)
                 rednet.send(3, "Hello")
             end, { open = true })
             local computer_2, modem_2 = computer_with_rednet(2, nil, { open = true, rep = true })
@@ -171,7 +172,7 @@ describe("The rednet library", function()
 
         it("repeats messages between computers with massive ids", function()
             local id_1, id_3 = 24283947, 93428798
-            local computer_1, modem_1 = computer_with_rednet(id_1, function(rednet, _ENV)
+            local computer_1, modem_1 = computer_with_rednet(id_1, function(rednet)
                 rednet.send(id_3, "Hello")
                 local id, message = rednet.receive()
                 expect { id, message }:same { id_3, "World" }
@@ -187,5 +188,39 @@ describe("The rednet library", function()
 
             fake_computer.run_all({ computer_1, computer_2, computer_3 }, { computer_1, computer_3 })
         end)
+
+        it("ignores duplicate messages", function()
+            local computer_1, modem_1 = computer_with_rednet(1, function(rednet)
+                rednet.send(2, "Hello")
+            end, { open = true })
+            local computer_2, modem_2 = computer_with_rednet(2, function(rednet, env)
+                local id, message = rednet.receive()
+                expect { id, message }:same { 1, "Hello" }
+
+                local id = rednet.receive(nil, 1)
+                expect(id):eq(nil)
+
+                env.sleep(10)
+
+                -- Ensure our pending message store is empty. Bit ugly to prod internals, but there's no other way.
+                expect(debugx.getupvalue(rednet.run, "tReceivedMessages")):same({})
+                expect(debugx.getupvalue(rednet.run, "nClearTimer")):eq(nil)
+            end, { open = true })
+
+            local computer_3, modem_3 = computer_with_rednet(3, nil, { open = true, rep = true })
+            fake_computer.add_modem_edge(modem_1, modem_3)
+            fake_computer.add_modem_edge(modem_3, modem_2)
+
+            local computer_4, modem_4 = computer_with_rednet(4, nil, { open = true, rep = true })
+            fake_computer.add_modem_edge(modem_1, modem_4)
+            fake_computer.add_modem_edge(modem_4, modem_2)
+
+            local computers = { computer_1, computer_2, computer_3, computer_4 }
+            fake_computer.run_all(computers, false)
+            fake_computer.advance_all(computers, 1)
+            fake_computer.run_all(computers, { computer_1 })
+            fake_computer.advance_all(computers, 10)
+            fake_computer.run_all(computers, { computer_1, computer_2 })
+        end)
     end)
 end)

src/test/resources/test-rom/spec/support/debug_ext.lua

@@ -0,0 +1,9 @@
+local function getupvalue(fn, name)
+    for i = 1, debug.getinfo(fn, "u").nups do
+        local up_name, value = debug.getupvalue(fn, i)
+        if up_name == name then return value end
+    end
+    error("Cannot find upvalue with name " .. name, 2)
+end
+
+return { getupvalue = getupvalue }

src/test/resources/test-rom/spec/support/fake_computer.lua

@@ -7,7 +7,7 @@ end
 local safe_globals = {
     "assert", "bit32", "coroutine", "debug", "error", "fs", "getmetatable", "io", "ipairs", "math", "next", "pairs",
     "pcall", "print", "printError", "rawequal", "rawget", "rawlen", "rawset", "select", "setmetatable", "string",
-    "table", "term", "tonumber", "tostring", "type", "utf8", "xpcall",
+    "table", "term", "textutils", "tonumber", "tostring", "type", "utf8", "xpcall",
 }
 
 --- Create a fake computer.
@@ -15,6 +15,7 @@ local function make_computer(id, fn)
     local env = setmetatable({}, _G)
 
     local peripherals = {}
+    local pending_timers, next_timer, clock = {}, 0, 0
     local events = { { n = 1, env } }
     local function queue_event(...) events[#events + 1] = table.pack(...) end
 
@@ -40,9 +41,18 @@ local function make_computer(id, fn)
             if event_data[1] == "terminate" then error("Terminated", 0) end
             return table.unpack(event_data, 1, event_data.n)
         end,
-        startTimer = function() return 0 end,
+        startTimer = function(delay)
-        clock = function() return 0 end,
+            local t = next_timer
+            pending_timers[t], next_timer = clock + delay, next_timer + 1
+            return t
+        end,
+        clock = function() return clock end,
+        sleep = function(time)
+            local timer = env.os.startTimer(time or 0)
+            repeat local _, id = env.os.pullEvent("timer") until id == timer
+        end,
     }
+    env.sleep = env.os.sleep
     env.dofile = function(path)
         local fn, err = loadfile(path, nil, env)
         if fn then return fn() else error(err, 2) end
@@ -67,7 +77,17 @@ local function make_computer(id, fn)
         end
     end
 
-    return { env = env, peripherals = peripherals, queue_event = queue_event, step = step, co = co }
+    local function advance(dt)
+        clock = clock + dt
+        for id, clk in pairs(pending_timers) do
+            if clk <= clock then
+                queue_event("timer", id)
+                pending_timers[id] = nil
+            end
+        end
+    end
+
+    return { env = env, peripherals = peripherals, queue_event = queue_event, step = step, co = co, advance = advance }
 end
 
 local function parse_channel(c)
@@ -137,17 +157,24 @@ local function run_all(computers, require_done)
 
         for _, computer in pairs(computers) do
             if coroutine.status(computer.co) ~= "dead" and (type(require_done) ~= "table" or require_done[computer]) then
-                error(debug.traceback(computer.co, "Computer did not shutdown"), 0)
+                error(debug.traceback(computer.co, ("Computer #%d did not shutdown"):format(computer.env.os.getComputerID())), 0)
             end
         end
     end
 end
 
+--- Advance all computers by a given time.
+local function advance_all(computers, dt)
+    for _, computer in pairs(computers) do computer.advance(dt) end
+end
+
 return {
     make_computer = make_computer,
     add_modem = add_modem,
     add_modem_edge = add_modem_edge,
     add_api = add_api,
+
     step_all = step_all,
     run_all = run_all,
+    advance_all = advance_all,
 }