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