Is there an easy way to audit our code for use of functions as keys?
I’m thinking I may have accidentally done this and I’m not sure how to audit my code for usage, as I have a lot of code.
5 Likes
This can already be done though.
local isCancelled = false
task.delay(5, function()
if isCancelled then return end
-- do stuff
end)
You could write a wrapper for this as well.
1 Like
Nice to see the idea of Lua 5.3 caching closure objects with same up values being implemented in Luau finally…
1 Like
(post marked for deletion for privacy reasons)
2 Likes
Unfortunately we don’t know how to automatically flag this, auditing this by hand may be tedious because you’ll have to read all of your code 
An easy recommendation is to test the code in Studio since the change is already live there, but if you have a lot of framework code that doesn’t come with tests I don’t know if there’s a great option…
4 Likes
All of my code appears to work, so I’m just worried about edge cases where I’m using functions as an index for cleanup in a maid class.
I’ll probably just throw a warning into the maid class if you try to use a function as a key.
Chances are this only really breaks in race conditions, so that‘a why I’m concerned. 
4 Likes
I believe it’s not same-upvalues-caching but rather that your example code falls under the condition of top level upvalues being unmodified.
Obsere how your code with 1 line added at the bottom changes the output to false, even though the actual logic remains unchanged:
local function b()
return function() print(b) end
end
print(b() == b())
b = b
I believe it’s not same-upvalues-caching but rather that your example code falls under the condition of top level upvalues being unmodified.
Correct, that is what I meant by same up values.
I have a serious concern. I use “callbacks” a lot in projects that are based on tables of functions like:
t = {[“myfunc1”] = function() … end, [“myfunc2”] etc etc}
and then execute the functions independently (based on whatever logic, to keep them synchronized).
From what I read, because they use the same key I will no longer be able to execute them independently.
The game is live with over 20K users. The amount of work is insane. I’m genuinely scared lol
If you could please provide a list with all possible complications you’ve found so we won’t have to work on weekends, that would be great.
1 Like
It is still much more performant to use the second. The first one would not be affected by this change because self is an upvalue not in the top-level scope, you would still need to use method functions which converts self into an argument (but you should use method functions for class objects anyways).
The first one becomes more memory efficient than it was before, and likely has less allocation overhead, but it will still likely not be as fast or efficient as the setmetatable version since you are uniquely allocating to the self table, and thus you’re using more memory and doing more assignments per object.
4 Likes
This will not effect your code, your functions will still work independently, so, no worries.
The case mentioned in the post is about having functions as keys (e.g. t[function() end] = "something"). When code that assigns or gets functions in this way the functions will start sharing the same object if they have the same source code and source location. (You might see function: 0x1 and function: 0x2 for two functions with identical code and no upvalues, but after this change they will both be function: 0x1)
Example demonstrating how behaviour can change:
local function getAbc()
local function abc()
return "something"
end
return abc
end
local t = {}
t[getAbc()] = 123
t[getAbc()] = 123
t[getAbc()] = 123
print(t)
-- Before this change t would look like this, three unique functions are created:
{
[function: 0x1] = 123,
[function: 0x2] = 123,
[function: 0x3] = 123
}
-- After this change t will look like this because the same function object is returned each time:
{
[function: 0x1] = 123
}
To explain a little more, what you do already only creates the function once so this change wouldn’t actually effect that code you wrote at all.
4 Likes
Quick question, will
Button.MouseButton1Click:Connect(function(...)
not work anymore?
1 Like
As long as you are not comparing the function to another function, it should be fine.
1 Like
That is what I am confused about. If I am doing
Button.MouseEnter:Connect(function(...)
and on another line, I do
Button.MouseLeave:Connect(function(...)
It sounds like in your statement that one of the lines wouldn’t work.
No, a problem could only occur if you used a comparator (such as ==) on the functions, for example:
The following is fine:
Button.MouseEnter:Connect(function(...)
end)
Button.MouseLeave:Connect(function(...)
end)
But the following may cause errors (well, different behaviour at least): (from what I have read)
function foo(arg)
return function() print(arg) end
end
if foo() == foo() then
end
2 Likes
Is this change still coming live today after the outage?
We’ll enable this on November 3rd (Wednesday) to be safe.
2 Likes
Out of curiousity, do you have any estimates on how much of an improvement this is? (Either for an average experience or specifically an average chunk of code where the optimization would be most useful)
It’s very difficult to tell on average. This makes one of our benchmarks (non-artificial, eg it isn’t specifically measuring function allocation time) 15% faster, but it would not have any difference if your code isn’t allocating function objects already.
3 Likes
In a situation like this, where the compiler would decide to share this function:
local t = function() end
t = nil
Would this function be garbage collected? Or would it exist in memory forever now that it has been created, incase it needs to be used again if another identical function is created?