-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Filling array from offset can be slow #590
Comments
Yeah there's an edge case in the table invariant that we knew about, but didn't have a sufficiently motivating use case to fix. I think this only applies when the offset is large/comparable with the target array size, as is in this example. (as an aside, your timings are a little surprising, on my system Lua 5.4 produces roughly comparable timings to Luau for the example that is starting from 1 - although if this is on macOS/M1 then yeah Lua 5.4 gets some benefits from first-class integers in programs like this, which we plan to address in the future) |
I use x86_64. This could be caused by building Luau with just |
Ah, gotcha - yeah, makes sense in this case. There's a few simple tweaks to table resize that eliminate this edge case, so we'll take a look - thanks! |
I have a few adjustments ready that will bring time back to 10s of milliseconds. |
Ok, it seems that it's harder to get right than I expected, so probably won't have a fix. |
Found the right fix, but unfortunately it will miss this weeks sync. |
* A small subset of control-flow refinements have been added to recognize type options that are unreachable after a conditional/unconditional code block. (Fixes #356). Some examples: ```lua local function f(x: string?) if not x then return end -- x is 'string' here end ``` Throwing calls like `error` or `assert(false)` instead of 'return' are also recognized. Existing complex refinements like type/typeof and tagged union checks are expected to work, among others. To enable this feature, `LuauTinyControlFlowAnalysis` exclusion has to be removed from `ExperimentalFlags.h`. If will become enabled unconditionally in the near future. * Linter has been integrated into the typechecker analysis so that type-aware lint warnings can work in any mode `Frontend::lint` methods were deprecated, `Frontend::check` has to be used instead with `runLintChecks` option set. Resulting lint warning are located inside `CheckResult`. * Fixed large performance drop and increased memory consumption when array is filled at an offset (Fixes #590) * Part of [Type error suppression RFC](https://github.com/Roblox/luau/blob/master/rfcs/type-error-suppression.md) was implemented making subtyping checks with `any` type transitive. --- In our work on the new type-solver: * `--!nocheck` mode no longer reports type errors * New solver will not be used for `--!nonstrict` modules until all issues with strict mode typechecking are fixed * Added control-flow aware type refinements mentioned earlier In native code generation: * `LOP_NAMECALL` has been translated to IR * `type` and `typeof` builtin fastcalls have been translated to IR/assembly * Additional steps were taken towards arm64 support
* A small subset of control-flow refinements have been added to recognize type options that are unreachable after a conditional/unconditional code block. (Fixes luau-lang/luau#356). Some examples: ```lua local function f(x: string?) if not x then return end -- x is 'string' here end ``` Throwing calls like `error` or `assert(false)` instead of 'return' are also recognized. Existing complex refinements like type/typeof and tagged union checks are expected to work, among others. To enable this feature, `LuauTinyControlFlowAnalysis` exclusion has to be removed from `ExperimentalFlags.h`. If will become enabled unconditionally in the near future. * Linter has been integrated into the typechecker analysis so that type-aware lint warnings can work in any mode `Frontend::lint` methods were deprecated, `Frontend::check` has to be used instead with `runLintChecks` option set. Resulting lint warning are located inside `CheckResult`. * Fixed large performance drop and increased memory consumption when array is filled at an offset (Fixes luau-lang/luau#590) * Part of [Type error suppression RFC](https://github.com/Roblox/luau/blob/master/rfcs/type-error-suppression.md) was implemented making subtyping checks with `any` type transitive. --- In our work on the new type-solver: * `--!nocheck` mode no longer reports type errors * New solver will not be used for `--!nonstrict` modules until all issues with strict mode typechecking are fixed * Added control-flow aware type refinements mentioned earlier In native code generation: * `LOP_NAMECALL` has been translated to IR * `type` and `typeof` builtin fastcalls have been translated to IR/assembly * Additional steps were taken towards arm64 support
When filling an array from a larger offset filling the array can become very slow.
The following code finished in PUC Lua and LuaJIT in under a second while Luau needs almost 3 minutes!
However, when beginning from index 1 all implementations are comparable.
The following table shows the timings of the two cases for different Lua implementations.
The problem seems to be that at some point the insertions start to hit the case https://github.com/Roblox/luau/blob/a934f742d81aefa53e8e45fa7eec0042d3fde7fc/VM/src/ltable.cpp#L530-L536 which rehashes the whole array every time the next index is inserted.
The text was updated successfully, but these errors were encountered: