From 7bb7b5e638f9dc4b95c85aad38370bb43b27bc1e Mon Sep 17 00:00:00 2001 From: JackMacWindows Date: Sun, 26 Sep 2021 16:15:37 -0400 Subject: [PATCH] Make Rednet deduplication more efficient (#925) --- .../computercraft/lua/rom/apis/rednet.lua | 23 ++++---- .../test-rom/spec/apis/rednet_spec.lua | 53 +++++++++++++++---- .../test-rom/spec/support/debug_ext.lua | 9 ++++ .../test-rom/spec/support/fake_computer.lua | 37 +++++++++++-- 4 files changed, 97 insertions(+), 25 deletions(-) create mode 100644 src/test/resources/test-rom/spec/support/debug_ext.lua diff --git a/src/main/resources/data/computercraft/lua/rom/apis/rednet.lua b/src/main/resources/data/computercraft/lua/rom/apis/rednet.lua index b6e478e7a..db40ebeac 100644 --- a/src/main/resources/data/computercraft/lua/rom/apis/rednet.lua +++ b/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 - tReceivedMessageTimeouts[os.startTimer(30)] = nMessageID + tReceivedMessages[nMessageID] = os.clock() + 9.5 + 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 - tReceivedMessageTimeouts[os.startTimer(30)] = tMessage.nMessageID + tReceivedMessages[tMessage.nMessageID] = os.clock() + 9.5 + 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 - local nMessage = tReceivedMessageTimeouts[nTimer] - if nMessage then - tReceivedMessageTimeouts[nTimer] = nil - tReceivedMessages[nMessage] = nil + nClearTimer = nil + local nNow, bHasMore = os.clock(), nil + for nMessageID, nDeadline in pairs(tReceivedMessages) do + if nDeadline <= nNow then tReceivedMessages[nMessageID] = nil + else bHasMore = true end end + nClearTimer = bHasMore and os.startTimer(10) end end end diff --git a/src/test/resources/test-rom/spec/apis/rednet_spec.lua b/src/test/resources/test-rom/spec/apis/rednet_spec.lua index f2fe84b0b..de5a85b60 100644 --- a/src/test/resources/test-rom/spec/apis/rednet_spec.lua +++ b/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 fns = { _ENV.rednet.run } + local computer = fake_computer.make_computer(id, function(env) + 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.os.queueEvent("x") _ENV.os.pullEvent("x") + env.rednet.open("back") + 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) diff --git a/src/test/resources/test-rom/spec/support/debug_ext.lua b/src/test/resources/test-rom/spec/support/debug_ext.lua new file mode 100644 index 000000000..10b9b89c3 --- /dev/null +++ b/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 } diff --git a/src/test/resources/test-rom/spec/support/fake_computer.lua b/src/test/resources/test-rom/spec/support/fake_computer.lua index b08ad77b1..09f2c6223 100644 --- a/src/test/resources/test-rom/spec/support/fake_computer.lua +++ b/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, - clock = function() return 0 end, + startTimer = function(delay) + 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, }