Skip to content

Add fast path for integer strings in variable name comparator#8483

Open
sovdeeth wants to merge 2 commits intoSkriptLang:dev/featurefrom
sovdeeth:patch/improve_var_name_comparator
Open

Add fast path for integer strings in variable name comparator#8483
sovdeeth wants to merge 2 commits intoSkriptLang:dev/featurefrom
sovdeeth:patch/improve_var_name_comparator

Conversation

@sovdeeth
Copy link
Member

@sovdeeth sovdeeth commented Mar 4, 2026

Problem

Setting large lists is still quite a lot slower than it really should be. Setting a 1000 integer list 10,000 times on my pc takes 30 seconds. That should take less than a second, ideally. This is due to variables being backed by a sorted TreeMap with a custom comparator, which is called multiple times for every get/set relating to lists.

Solution

Adds a faster path in the custom comparator, which handles pure integer indices about twice as fast. It adds slight overhead to non-int indices, but since it fails fast it's only really relevant for lists with mixed indices and indices like 1235a, which are likely rather rare. My test case went from 30s to 12.7s to run.

I originally was going to optimize the full path, but to get the same performance made the code very hard to read and maintain, and for not a ton of benefit, since int indices are by far the most common.

Also adds tests for index sorting. Some of the behavior is really really weird. Why does a-2 come before a-1? Because the -2 is smaller than the -1!!! of course!!! don't ask why 0001 is larger than 1!

Part 2: Made splitVariableName much faster by throwing out regex and using indexOf instead.

Testing Completed

list ordering.sk

Supporting Information

Benchmark:

    set {_time} to now
    
    loop 10_000 times:
        set {_list1::*} to integers from 1 to 1000

        set {_list2::*} to integers from 1 to 100

        set {_copy1::*} to {_list1::*}
        set {_copy2::*} to {_list2::*}

    broadcast "%difference between now and {_time}%"

Completes: none
Related: none
AI assistance: Claude code, for initial optimization possibilities and for improvements to the indexOf splitting method.

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Mar 4, 2026
@sovdeeth sovdeeth requested review from a team as code owners March 4, 2026 22:48
@sovdeeth sovdeeth requested review from UnderscoreTud and cheeezburga and removed request for a team March 4, 2026 22:48
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Mar 4, 2026
@sovdeeth sovdeeth changed the base branch from master to dev/feature March 4, 2026 22:49
@sovdeeth sovdeeth force-pushed the patch/improve_var_name_comparator branch from 0965004 to f6fac44 Compare March 5, 2026 03:53
@sovdeeth sovdeeth moved this to In Review in 2.15 Releases Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Feature request, an issue about something that could be improved, or a PR improving something. needs reviews A PR that needs additional reviews

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

1 participant