Skip to content

Implement index tracking to optimize list operations#8519

Open
UnderscoreTud wants to merge 6 commits intodev/patchfrom
patch/optimize-list-adding
Open

Implement index tracking to optimize list operations#8519
UnderscoreTud wants to merge 6 commits intodev/patchfrom
patch/optimize-list-adding

Conversation

@UnderscoreTud
Copy link
Copy Markdown
Member

@UnderscoreTud UnderscoreTud commented Apr 1, 2026

Problem

When adding an element to a list, Skript scans through the entire list to find an open spot for the element. This is a pretty slow operation with a time complexity of O(n), and adds up extremely fast, to the point where adding 10k elements to a list takes around 11 seconds on my machine.

Deleting a list using delete {_list::*} loops through every index and removes it individually. This is done to ensure all sublists are also deleted, but this is also done even if the list doesn't have any sublists!.
Also for some reason the logic for deleting a list is effectively this:

  1. loop every index and delete its sublists
  2. set the list to null (which does the same thing as above)

So, deleting a list actually deletes it twice

Solution

Adding

Use a custom TreeMap implementation that tracks the list's numerical indices instead of scanning the whole list every time something gets added. The map keeps track of the next open numerical index as elements are inserted and removed, so adding to a list no longer has to walk through every existing entry looking for a gap.

In the common case where the list indices are consecutive, adding an element just appends it to the end immediately. If there are gaps from deleted entries, the map reuses the lowest available numerical index instead. This brings list addition down from O(n) to effectively O(1) in the best case, while still avoiding the full scan that made the old design so slow.

With this implementation, adding 1 million elements to a list takes ~1.5 seconds on my machine, that's over a 70,000% improvement over the old design!

Deleting

Deleting a list now sets it to null, and the TreeMap implementation now keeps track of the sublist indices, so it only loops over the necessary indices rather than walking the entire list each time

Testing Completed

IndexTrackingTreeMapTest.java

Supporting Information

Benchmarks:

Benchmark Before After Improvement%
Adding 11.5 seconds (10 thousand elements) 1.59 seconds (1 million elements) 72,327%
Setting 2.27 seconds 2.11 seconds 107% (Roughly equal, within margin of error)
Deleting entire list 1.07 seconds 0.08 seconds 1,337%

Completes: #891
Related: #891
AI assistance: Junie was used to generate the majority of test cases

@UnderscoreTud UnderscoreTud requested review from a team as code owners April 1, 2026 16:49
@UnderscoreTud UnderscoreTud requested review from APickledWalrus and TheMug06 and removed request for a team April 1, 2026 16:49
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Apr 1, 2026
sovdeeth

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@Absolutionism Absolutionism left a comment

Choose a reason for hiding this comment

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

Solid 💪

@skriptlang-automation skriptlang-automation bot added patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. and removed needs reviews A PR that needs additional reviews labels Apr 1, 2026
Copy link
Copy Markdown
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

looking great

@skriptlang-automation skriptlang-automation bot removed the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Apr 3, 2026
@UnderscoreTud UnderscoreTud requested a review from sovdeeth April 3, 2026 17:03
@skriptlang-automation skriptlang-automation bot added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Apr 3, 2026
@sovdeeth sovdeeth added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Apr 3, 2026
@sovdeeth sovdeeth moved this to In Review in 2.15 Releases Apr 3, 2026
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.15 Releases Apr 3, 2026
@sovdeeth sovdeeth removed the status in 2.15 Releases Apr 3, 2026
@sovdeeth sovdeeth moved this to Awaiting Merge in 2.15 Releases Apr 3, 2026
@UnderscoreTud UnderscoreTud changed the title Implement numerical index tracking to optimize adding to lists Implement index tracking to optimize list operations Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.

Projects

Status: Awaiting Merge

Development

Successfully merging this pull request may close these issues.

4 participants