Skip to content
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

feat(runtime): Optimize == for lists #2247

Merged
merged 7 commits into from
Feb 17, 2025

Conversation

spotandjake
Copy link
Member

@spotandjake spotandjake commented Feb 17, 2025

This optimizes == for specifically for lists and also refactors it to use early return. Instead of the recursive approach used in the general case for adt's because we know lists will always have two elements (item, nextItemPtr) we handle it in an iterative manner avoiding the stack overflow.

Notes:

  • The smallest program size increased because == is bundled to every grain program.
  • == would now lock with a cyclic list, however from my understanding such a list would be impossible to build in grain.
    • If we want to handle cycles we can but it adds overhead as we need to go back through the scanned items and do the unmarking after.
    • This will all need to get refactored with the wasm gc changes we plan to make so I think it's fine to take the perf improvement for now.
  • We should be able to support any list size now however it seems we start running into issues with List.init around 5005 elements.

Closes: #2246

Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

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

Should we add a test with a very very long list?

@spotandjake spotandjake force-pushed the spotandjake-eq-lst-opt branch from 874d4de to 44e2f73 Compare February 17, 2025 05:10
@spotandjake
Copy link
Member Author

I added that test and this should be ready to merge now.

@ospencer ospencer added this pull request to the merge queue Feb 17, 2025
Merged via the queue into grain-lang:main with commit 1cba005 Feb 17, 2025
12 checks passed
@github-actions github-actions bot mentioned this pull request Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack overflow on equality comparsions of large lists
2 participants