Closed tomlau10 closed 4 months ago
Contributions welcome. I've never seen a speed regression on this scale, but obviously we'd be happy to see it fixed. I probably won't have time to work on it for a while, but perhaps you or anybody else that comes along can come up with a solution. I'll be happy to facilitate contributions and getting them released.
Two approaches I can think of to look into:
- If the answer to some question is just being recalculated from the same inputs over and over, it might work to just add memoization to the function.
It looks like a solid approach! After reviewing the code for contains_call
, it seems that caching the result into node.is_contains_call
before the function returns could be a viable solution. To elaborate, the function would return the value of node.is_contains_call
at the beginning if it is not nil.
Perhaps something like this:
local function contains_call(node)
if node.is_contains_call ~= nil then
-- return cached result
return node.is_contains_call
end
node.is_contains_call = false
if node.tag == "Call" or node.tag == "Invoke" then
node.is_contains_call = true
elseif node.tag ~= "Function" then
for _, sub_node in ipairs(node) do
if type(sub_node) == 'table' and contains_call(sub_node) then
node.is_contains_call = true
break
end
end
end
return node.is_contains_call
end
I might test it next week if I have time and open a pull request after testing 😄
I've just tested the above changes on latest version of luacheck locally and confirmed that it now has the same performance as v0.25
. I'll soon open a PR on it. 🎉
TL;DR
The introduction of the
is_circular_reference
function call within the for loop ofstages/resolve_locals.lua
since versionv0.26
seems to significantly impact performance as observed in this commit: a24332eThe long story
Greetings, I've been utilizing luacheck
v0.25
vialuarocks
in a private project for an extended period. Within this project, there exists a sizable Lua file (>100K lines) primarily comprising data tables (~95%) interspersed with logic code (~5K lines). Under versionv0.25
, linting this file takes approximately 10 seconds, which is acceptable given its substantial size.However, subsequent to updating to the latest release, luacheck
v1.1.2
, the linting process for this file consumes over 10 minutes. Further investigation revealed that this performance regression commenced withv0.26
.Upon analyzing the differences between v0.25 and v0.26, I attempted to revert the alterations near the aforementioned for loop. Consequently, the linting time returned to normal, averaging around ~10 seconds.
How to reproduce
Given the private nature of the project, sharing the actual file is not feasible. Instead, I will outline the file structure and provide a minimal test file capable of reproducing the issue.
File Structure:
Observations
local DATA = {...}
portion, leaving only the local function definitions (~5K lines), results in a lint time of ~1 second for both luacheck versions.local function
definitions, leaving only thelocal DATA = {...}
(~95K lines), yields a lint time of ~8 seconds for both luacheck versions.v0.26
to take 10+ minutes.on_use
functions from the data templates, such that none of them have upvalues pointing above, leads to a lint time of ~1 minute for luacheckv0.26
.Minimal Reproducing File
I have crafted a
gen_test.lua
script to generate atest.lua
file in the aforementioned format:On my MacBook Pro 2016,
v0.25
takes less than 1 second whilev0.26
takes around 30 seconds.Thoughts
I acknowledge that this file structure may be unconventional, but refactoring is challenging within the project. I aim to lint this file given the presence of logic code in the upper portion and the inline
on_use
functions in the data templates. Are there alternative suggestions on how to work around or disable the newly added circular reference check? Could theis_circular_reference()
function be optimized, perhaps by caching the result of a checked node?Your assistance in addressing this performance concern would be greatly appreciated.