Skip to content

Optimize string.split - #2981

Merged
vrn-sn merged 2 commits into
luau-lang:masterfrom
gemtool:optimize-string-split
Sep 24, 2026
Merged

vrn-sn merged 2 commits into
luau-lang:masterfrom
gemtool:optimize-string-split

Conversation

@gemtool

@gemtool gemtool commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

string.split called memcmp at every byte offset and inserted results with lua_pushinteger + lua_settable into an empty table. This change:

  • uses memchr for single-character separators, counting the pieces first so the result table is allocated at its final size
  • for multi-character separators, compares the first and last characters inline and only calls memcmp when both match, so no position does more work than before
  • presizes the result for the empty separator, and uses lua_rawseti throughout

Output is unchanged: a differential fuzz of 20k random inputs, including embedded NULs and overlapping separators, matches the old implementation exactly. It also removes a pointer computation before the start of the buffer when the separator is longer than the string.

bench/micro_tests/test_string_split.lua before after speedup
short csv 67.9 ms 37.5 ms 1.81x
long lines 37.0 ms 16.7 ms 2.22x
long lines, 2-char separator 36.9 ms 17.3 ms 2.13x
empty separator 21.2 ms 12.6 ms 1.68x
separator prefix repeats 25.8 ms 7.8 ms 3.31x
separator prefix and suffix repeat (worst case) 25.8 ms 25.8 ms 1.00x

The last row is a deliberately adversarial input where both inline checks pass at every position; it performs the same memcmp as before and runs at parity.

Added conformance tests for edge cases (embedded NULs, overlapping separators, separator longer than the input).

@gemtool
gemtool requested a review from a team as a code owner September 23, 2026 09:55
@gemtool
gemtool requested a review from vrn-sn September 23, 2026 09:55

@vrn-sn vrn-sn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! Could you please flag your changes (explained in this file)?

I think this is a good use case for LUAU_DYNAMIC_FASTFLAGVARIABLE, which will let us dynamically enable/disable the flag if there does turn out to be a bug. We try to ensure the "flag off" branch is identical to the original code, even if this means there will be quite a bit of duplicated logic in str_split.

@vrn-sn

vrn-sn commented Sep 23, 2026

Copy link
Copy Markdown
Member

To keep it simple, I would probably just do something like this:

    size_t haystackLen;
    const char* haystack = luaL_checklstring(L, 1, &haystackLen);
    size_t needleLen;
    const char* needle = luaL_optlstring(L, 2, ",", &needleLen);

    if (DFFlag::YourStringSplitFlagName)
    {
        // your new logic
    }
    else
    {
        // the old logic, verbatim
    }

    return 1;

@gemtool

gemtool commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

@vrn-sn Done, thanks! The new logic is now behind DFFlag::LuauOptimizeStringSplit (declared with LUAU_DYNAMIC_FASTFLAGVARIABLE, default off), and the flag-off branch is the original implementation verbatim. Tests pass with flags at their defaults and with all flags enabled.

@vrn-sn vrn-sn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution - especially appreciated the benchmarks!

@vrn-sn
vrn-sn merged commit d42c8d5 into luau-lang:master Sep 24, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants