Lua Signal Class Comparison & Optimal `GoodSignal` Class

Here I provide a comparison of 4 different Signal class implementations, which each have various advantages and disadvantages, as well as presenting what I think is the optimal Signal class implementation to use going forwards.

TL;DR: Use this GoodSignal implementation (CODE LINK) unless you have a good reason not to!

Below I provide links to a Github gist of the code for each implementation, but if you want to download the full code for all of them + Unit Tests + Benchmark code you can get a Model containing all of that here: Signal class comparison with Tests & Benchmark - Roblox

API

All 4 variants share the following API:

local Signal = require(--[[the module]])
local sig = Signal.new()
local connection = sig:Connect(function(...)
    print(...)
end)
sig:Fire(nil, "test1") --> Invokes the handler, printing `nil test1`
connection:Disconnect()
sig:DisconnectAll() --> If you need to disconnect all
task.spawn(function()
    print(sig:Wait())
end)
sig:Fire(nil, "test2") --> Resumes the waiter, printing `nil test2`

RobloxSignal Implementation (link)

The most obvious implementation of a Signal class is this one: A simple wrapper around a BindableEvent. There’s some detail to correctly implementing this, because by default a BindableEvent deep-copies any tables that you pass to it, which you probably did not want. To work around this, you have to pass the arguments through some other mechanism and “pick them back up” in the event handler.

This is further complicated if you want the implementation to still work correctly with the new SignalBehavior = Deferred switch, which the implementation linked here actually does. Doing this requires another trick based on the fact that event handlers are called in reverse order of connection, but doesn’t add much additional cost.

SimpleSignal Implementation (link)

What comes next after an implementation directly on top of a BindableEvent? Implementing one in pure Lua of course! The SimpleSignal implementation here presents implementing a pure Lua Signal class which respects all of the behaviors that a normal RBXScriptSignal has, while using the most straightforward implementation possible.

What do we mean “respects all of the behaviors” here? Specifically I mean that:

  • It is “yield-safe” – you can yield inside of the event handlers without blocking progress of the thread which called :Fire(...).
  • Event handlers are run in reverse order of connection.
  • Connecting a new event handler in the middle of processing an existing event handler will not cause that new event handler to be run until subsequent calls to :Fire(...).
  • An event handler can :Disconnect() itself without causing issues.
  • A :Wait() method is provided.

Previously full correctness here was not possible, but the very recently added task.spawn API makes this Signal implementation possible.

FastSignal Implementation (link)

Next, we can go all the way in the other direction with FastSignal, which sacrifices all that correctness that SimpleSignal had in order to be as performant as possible while still implementing the Signal class patterns. Specifically, it sacrifices correctness in that:

  • Yielding an an event handler will cause the thread that called :Fire() to yield, so you need to explicitly call task.spawn in an event handler when it needs to make yielding function calls.
  • Connecting new event handlers in an event handler will cause unpredictable and problematic behavior (may cause existing event handlers to not be called correctly), though an event handler can still disconnect specifically itself without causing issues.

GoodSignal Implementation (link)

Where can we go from here? Well, next we have the main event, I present the GoodSignal implementation, which provides as much of the performance benefit of FastSignal as possible without sacrificing the correctness of SimpleSignal. (It manages to do this by using a much more complex implementation than SimpleSignal).

The main benefit of GoodSignal over SimpleSignal is that it has 2x or better performance in most scenarios, however it has also a couple of behavior improvements over the SimpleSignal implementation:

  • :Fire never causes any memory allocation unless you yield in an event handler, and :Connect only costs exactly one memory allocation.
  • the GoodSignal implementation will handle all cases of :Fireing and :Disconnecting in an event handler correctly. (The SimpleSignal implementation may fail when you disconnect other handler functions from within a handler function).

An Aside on Memory Leaks

When using one of the pure Lua implementions of Signal, you don’t have to worry at all about extra memory leaks. However, when using an implementation backed by a BindableEvent such as RobloxSignal, you have to be very careful to actually disconnect all of the connections you make, otherwise you will run into the following issue: PSA: Connections can memory leak Instances!

For example, the following will permanently leak memory (but would not if you used one of the pure Lua implementations):

do
    local sig = RobloxSignal.new()
    sig:Connect(function()
        print(sig)
    end)
end

Detailed Performance Comparison

Here is a comparison of the performance of the implementations above. Hopefully the names of the rows are self-explanitory:

                      | FastSignal | GoodSignal | SimpleSignal | RobloxSignal | 
--------------------------------------------------------------------------------
CreateAndFire         |  0.6μs     |  1.2μs     |  2.4μs       |  18.5μs      | 
ConnectAndDisconnect  |  0.3μs     |  0.3μs     |  0.4μs       |  1.8μs       | 
FireWithNoConnections |  0.1μs     |  0.0μs     |  0.0μs       |  2.2μs       | 
Fire                  |  0.2μs     |  0.8μs     |  3.8μs       |  3.2μs       | 
FireManyArguments     |  0.2μs     |  0.8μs     |  2.0μs       |  3.5μs       | 
FireManyHandlers      |  0.2μs     |  4.4μs     |  15.2μs      |  6.0μs       | 
FireYieldingHandler   |  N/A       |  5.1μs     |  5.1μs       |  6.1μs       | 
WaitOnEvent           |  3.1μs     |  3.5μs     |  5.1μs       |  5.6μs       | 

What Do Those Performance Results Mean?

  • GoodSignal performs better than SimpleSignal in almost every regard. The only case where it would ever make sense to use SimpleSignal is if you need to edit the Signal class adding more functionality, and aren’t comfortable trying to edit the significantly more complicated GoodSignal implementation.

  • FastSignal wins by a lot on performance… but it comes with the cost of having potential bugs if you change something from a yielding call to a non-yielding call without thinking about it or end up accidentally disconnecting handlers in a scenario you didn’t think of ahead of time and breaking the behavior. If you really need the perf it makes sense to use FastSignal, however you should keep in mind that as soon as you start writing non-trivial code inside of an event handler, the cost of the code in the event handler will dwarf the raw cost of firing the event handler, and make the perf cost of the Signal implementation not very relevant.

  • There’s almost no reason to use RobloxSignal now that task.spawn exists allowing better pure Lua signal implementations: It has worse perf in every regard and introduces memory leak potential. The only reason might be if you’re really paranoid about wanting exactly the same behavior as RBXScriptSignal no matter what happens.

A note on Array vs Dict based Objects

Some people have suggested using an array to store the fields of the Signal / Connections objects internally instead of a dict. This is not a good idea. Here’s a performance comparison between using an array vs a dict (GoodSignal) internally. The dict wins:

image

This may be somewhat surprising to people who have seen Lua optimization advice. That’s because the only applies to Roblox’ Luau VM! In vanilla Lua using an array internally rather than a dict with fields would be almost always be faster, but the Luau VM has some optimizations which make field access very fast, even faster than array access in some conditions like these.

Closing

The task library is awesome! Not much more to add than that.

105 Likes

Looks great! Question: Why do task.spawn against a coroutine and not just the function itself? Also, any opinions on using defer vs spawn?

6 Likes

Better perf. The task library is low level and does exactly what you tell it to do, if you pass it a function it will create a new coroutine to run it in every time you call it.

That’s one of the things that makes GoodSignal faster than SimpleSignal, SimpleSignal directly task.spawns the handler function, vs GoodSignal re-uses the same coroutine over and over again for running handlers unless you actually yield in the handler.

There’s no reason to use the old spawn unless you actually want the janky builtin task scheduler resumption throttling: Resuming waits / spawns is only allowed to take up to 10% of a frame’s time, further resumes / waits after that will be delayed until a future frame even if they “should” already be running given their delay value.

Comparatively, the task library does exactly what you tell it to (if you task.defer something then it’s 100% going to be run this frame), and you have to handle the consequences of that yourself, which is the control that most devs want.

If what you’re asking is whether to use task.spawn or task.defer… they serve different purposes. Most of the time it should not matter which you use but task.spawn is a bit easier to reason about in my opinion, so you should probably gravitate towards that unless you’re doing something more complicated and have a reason to use task.defer.

6 Likes

Great post, might switch my signal implantation over. I do think there is some cost to switching away from RobloxSignal, I have a set of code that checks explicitly for RBXSignalConnection and these will need to be changed. Also being agnostic about type of signals is nice. I do query Connection.IsConnected, but this should be easy to add to an implementation.

Can you try benchmarking the RobloxSignal implementation if you pass in a guid or an int over a closure? I’m guessing the closure is more expensive.

3 Likes

You guess wrong! What makes closures expensive is normally the upvalues references. A closure that doesn’t capture anything is actually very cheap, it only costs a single (pretty small) memory allocation.

I can get you benchmark values later to demonstrate though.

Worth noting that if you’re doing this as a micro-optimization for performance reasons, there’s no reason to do so with a Lua Signal implementation. Firing a signal with no connections to it is basically pretty close to free with a Lua implementation.

2 Likes

I’m querying it to determine if I should connect to the signal, or if a previous caller has disconnected already. Good news is this code is pretty much centralized in one location.

So goodsignal performs better than simplesignal, works fully as the simplesignal implementation, and fixes (reason i’m asking is it said it balances correctness and performance so I thought it would have less correctness than the simplesignal)

If so that’s great!

Yeah, GoodSignal does it all: GoodSignal is basically strictly better than SimpleSignal in that regard. The performance/correctness tradeoff is compared to FastSignal, I’ve edited those sections to be written in the opposite order to make things more clear.

1 Like

Looks good, I’m interested in learning about some of the language/implementations used here.

I’m looking to use (or simulate) immediate mode behaviour for certain signals when deferred mode becomes standard. I would assume it’d be trivial to adopt the pattern you use with RobloxSignal for other Roblox instance events? For example, being able to implement a fromRBXScriptSignal method that uses another event (e.g. Instance.Changed) instead of BindableEvent.Event.

I believe you can make :Disconnecting faster on GoodSignal by having a _prev value on each connection instead of looping through every connection to find it.

Unfortunately not. You can turn an immediate event into a deferred one, but you can’t go back in the other direction and turn a deferred event into an immediate one because that would imply your code getting called sooner than it was notified that something happened… unless you have a time-machine that’s not happening.

The way it works is actually the optimization already: The most common case for signals is there being exactly one handler connected to a given signal, or at most a few handlers, and the current implementation is designed to make that case fast. You could modify the implementation to handle the case where there’s a ton of connected handlers that must be individually disconnected faster, but it would come at the expense of making the common case slower because you’d need extra logic for managing the _prev reference.

2 Likes

Is there a specific reason GoodSignal is strict, or is it just for general good practice? It clashes with the maid and janitor classes so I intend on removing the strict behaviour

1 Like

Yeah, it’s just there to give cleaner errors, but if you have a reason to remove it then it won’t cause any problems to do so.

Why does strict interrupt those patterns? Does it look for a specific function to call inside the object? I thought Quenty’s implementation used typeof / metatable to distinguish stuff.

The janitor checks for the existence of a Destroy method when adding an object if you don’t specify the method to be called. Although I only just realised that the implementation of Signal that I was previously using actually just had a Destroy method, so I’ll need to change my code anyway.,

Yup. I’ll be forking GoodSignal most likely, and just renaming DisconnectAll() to Destroy so it fits my interface.

Disconnect will also need a Destroy method, but no big deal.

There was discussion above about the linked list implementation, but I’m curious why you used a linked list in the first place. Is it simply that it’s faster than table insertion, deletion, etc.?

The linked list has very good iteration stability while also being fast.

If you delete an entry out of an array, the index of each entry after it shifts. That means that any for loops that are in the middle of iterating over it may have their behavior messed up.

The SimpleSignal implementation solves this by copying the handler list and iterating over the copy instead.

You could also solve it by using a copy on write list of handlers, where you copy the handler list before you add or remove an entry.

But the intrusive linked list approach (intrusive in that the connections themselves serve as the links) solves the problem without any copy at all, without even sacrificing any performance in other ways. This works because even if a node is removed from the linked list (through the previous node no longer pointing to it), any loops sitting on that node can still follow the next pointer to get to the next node in the list.

5 Likes

Not sure how I missed this awesome thread but here’s some pure Lua function comparison / benchmark for people who have doubts and not convinced to use the new task library

(I’ll drop place file later)


@stravant

function Connection:Disconnect()
	assert(self._connected, "Can't disconnect a connection twice.", 2)

I don’t think this should error since RBXScriptConnection doesn’t iirc

The only thing that I disagree with is there is no Type Luau and GoodSignal doesn’t have exactly the same spec as RBXScriptConnection

Doesn’t have .Connected

Other than that it’s a really solid module I’m definitely forking it!

For reference, the only reason that Roblox doesn’t throw an error in that case is that there’s too much legacy code accidentally calling it more than once by doing this kind of pattern:

function onEquip/Select()
    if condition then
        onMouseDownConnection = mouse.Button1Down...
    end
end
function onUnequip()
    if onMouseDownConnection then
        onMouseDownConnection:Disconnect()
        --> forgot to set onMouseDownConnection to nil :(
    end
end

But disconnecting the same connection more than once is almost always a bug, so I chose to throw in my implementation.

I don’t really know why he didn’t just make it a public thing.

This is new behaviour, right? I remember this being default behaviour; I guess this is from Deferred Events launching…

I think it would be cool to see a little variable which defines that behaviour inside the script, but it’s whatever I guess.

If anyone is forking this and removing the coroutine recycling part, you can optimize :Wait in a way that you change _func to the :Wait's thread and then just resume it from there, you do have to keep some value to say that it is a wait thread, but it does work.

That’s better if you’re using arrays for private values, as arrays are O(1) to index, so the impact of searching that index is almost none.