Skip to content

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

Merged
APickledWalrus merged 3 commits intoSkriptLang:dev/featurefrom
sovdeeth:patch/improve_var_name_comparator
Mar 27, 2026
Merged

Add fast path for integer strings in variable name comparator#8483
APickledWalrus merged 3 commits intoSkriptLang:dev/featurefrom
sovdeeth:patch/improve_var_name_comparator

Conversation

@sovdeeth
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

dope

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.15 Releases Mar 27, 2026
@skriptlang-automation skriptlang-automation bot added feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. and removed needs reviews A PR that needs additional reviews labels Mar 27, 2026
@APickledWalrus APickledWalrus merged commit ccf431b into SkriptLang:dev/feature Mar 27, 2026
9 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done - Awaiting Release in 2.15 Releases Mar 27, 2026
@skriptlang-automation skriptlang-automation bot added completed The issue has been fully resolved and the change will be in the next Skript update. and removed feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Mar 27, 2026
sovdeeth added a commit that referenced this pull request Apr 2, 2026
* Add Origin-applying SyntaxRegistry (#8376)

* Add reduce/fold Expression (#8353)

* Location with Yaw/Pitch (#8351)

* Test framework: support directory resources (#8377)

* Test framework: support directory resources

* use walkFileTree

* oops forgot imports

* Pattern Stringification Customization (#8391)

* Implement StringificationProperties

* Package cleanup

* Fix missing call in GroupPatternElement stringification (#8449)

Fix GroupPatternElement stringification

* Stabilise damage source experiment (#8414)

* Stabilise damage source experiment

* Replace STABLE with MAINSTREAM

* Remove experiment from damage source test

* Remove unstable API usage supression

---------

Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>

* Add vector(n) (#8412)

* Init commit

* Fix not registering overloading for old namespaces

* Standardize Module usage + add child modules (#8346)

* child module support

* Create child modules and move over existing Bukkit addon modules.

* Update newer modules

* Clean up

* Requested changes and additional conversions

Just tags left!

* convert last module

* standardize package hierarchies

* remove bad imports

* fix bad examples

* Replace ChildAddonModule with HierarchicalAddonModule, move display/interaction modules into entity module, few other requested changes

* requested changes

* varargs registrar (thanks blue!)

* Use addonmodule javadocs to point to other useful tools

* Requested Changes

* Requested Changes

* Added option to compress file backups (#8396)

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

* Initial conversion

* Some more conversion

* fixes

* Update StructCommand.java

* no tests

---------

Co-authored-by: Patrick Miller <apickledwalrus@icloud.com>
Co-authored-by: mibers <midaswoah2@gmail.com>
Co-authored-by: _tud <98935832+UnderscoreTud@users.noreply.github.com>
Co-authored-by: devdinc <234956748+devdinc@users.noreply.github.com>
Co-authored-by: Tiberiu Sabău <96194994+tibisabau@users.noreply.github.com>
Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>
Co-authored-by: Pesek <42549665+Pesekjak@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something.

Projects

Status: Done - Awaiting Release

Development

Successfully merging this pull request may close these issues.

3 participants