Mercurial > prosody-hg
changeset 14198:73903352de86
util.smqueue: Refactor checkpoint logic and add tests
The previous checkpoint logic used recursion, and the code was more complex
than needed due to maintenance of the self._next_checkpoint field.
This switches to a loop, which should be a bit more efficient, partly because
there are fewer calls (although it was a tail call) and because table lookups
can now be cached between each execution.
_call_checkpoints() is now called unconditionally for every ack, which is
simpler though adds a function call, it should be more than balanced out by
the other changes.
| author | Matthew Wild <mwild1@gmail.com> |
|---|---|
| date | Tue, 26 May 2026 15:36:36 +0100 |
| parents | 3b26b8772fb1 |
| children | 91afc1f07337 |
| files | spec/util_smqueue_spec.lua util/smqueue.lua |
| diffstat | 2 files changed, 89 insertions(+), 15 deletions(-) [+] |
line wrap: on
line diff
--- a/spec/util_smqueue_spec.lua Tue May 26 12:31:32 2026 +0100 +++ b/spec/util_smqueue_spec.lua Tue May 26 15:36:36 2026 +0100 @@ -81,4 +81,85 @@ end end) end) + + describe("#add_checkpoint", function() + it("fires the callback when ack reaches the checkpoint", function() + local q = smqueue.new(10); + for i = 1, 4 do q:push(i); end + local called = false; + q:add_checkpoint(function() called = true; end); + assert.falsy(called, "should not fire before ack"); + q:ack(4); + assert.truthy(called, "should fire when ack reaches checkpoint"); + end) + + it("fires when ack exceeds the checkpoint", function() + local q = smqueue.new(10); + for i = 1, 4 do q:push(i); end + local called = false; + q:add_checkpoint(function() called = true; end); + for i = 5, 8 do q:push(i); end + q:ack(7); -- acks past the checkpoint at 4 + assert.truthy(called); + end) + + it("does not fire for acks below the checkpoint threshold", function() + local q = smqueue.new(10); + for i = 1, 5 do q:push(i); end + local called = false; + q:add_checkpoint(function() called = true; end); + q:ack(4); + assert.falsy(called, "should not fire when ack is below checkpoint"); + end) + + it("fires the callback with optional data argument", function() + local q = smqueue.new(10); + for i = 1, 4 do q:push(i); end + local received_data; + q:add_checkpoint(function(d) received_data = d; end, "hello"); + q:ack(4); + assert.equal("hello", received_data); + end) + + it("fires multiple checkpoints in a single ack call", function() + local q = smqueue.new(10); + for i = 1, 3 do q:push(i); end + local fired = {}; + q:add_checkpoint(function() fired[#fired+1] = "first"; end); + q:push(4); + q:add_checkpoint(function() fired[#fired+1] = "second-1"; end); + q:add_checkpoint(function() fired[#fired+1] = "second-2"; end); + q:push(5); + q:add_checkpoint(function() fired[#fired+1] = "third"; end); + q:ack(5); -- should call all checkpoints, in order + assert.same({ "first", "second-1", "second-2", "third" }, fired); + end) + + it("fires checkpoints in order across multiple acks", function() + local q = smqueue.new(10); + for i = 1, 3 do q:push(i); end + local order = {}; + q:add_checkpoint(function() order[#order+1] = 1; end); + q:push(4); + q:add_checkpoint(function() order[#order+1] = 2; end); + q:push(5); + q:add_checkpoint(function() order[#order+1] = 3; end); + q:ack(3); -- fires only first + assert.same({ 1 }, order); + q:ack(5); -- fires second and third + assert.same({ 1, 2, 3 }, order); + end) + + it("fires each checkpoint exactly once", function() + local q = smqueue.new(10); + for i = 1, 4 do q:push(i); end + local count = 0; + q:add_checkpoint(function() count = count + 1; end); + q:ack(4); + q:ack(4); -- duplicate ack + for i = 5, 8 do q:push(i); end + q:ack(8); + assert.equal(1, count); + end) + end) end);
--- a/util/smqueue.lua Tue May 26 12:31:32 2026 +0100 +++ b/util/smqueue.lua Tue May 26 15:36:36 2026 +0100 @@ -31,10 +31,8 @@ table.insert(acked, v); end - local c = self._next_checkpoint; - if c and h >= c then - self:_call_checkpoints(h); - end + self:_call_checkpoints(h); + return acked end @@ -68,22 +66,17 @@ function smqueue:add_checkpoint(cb, ud) table.insert(self._checkpoints, { self._head; cb; ud }); - if not self._next_checkpoint then - self._next_checkpoint = self._head; - end end function smqueue:_call_checkpoints(h) local checkpoints = self._checkpoints; - - local c = table.remove(checkpoints, 1); - self._next_checkpoint = checkpoints[1] and checkpoints[1][1] or nil; + local c = checkpoints[1]; - local cb, ud = c[2], c[3]; - cb(ud); - - if h >= self._next_checkpoint then - return self:_call_checkpoints(h) + while c and h >= c[1] do + table.remove(checkpoints, 1); + local cb, ud = c[2], c[3]; + cb(ud); + c = checkpoints[1]; end end
