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

Bucket typer delays #11465

Merged
merged 2 commits into from Jan 11, 2024
Merged

Bucket typer delays #11465

merged 2 commits into from Jan 11, 2024

Conversation

Simn
Copy link
Member

@Simn Simn commented Jan 5, 2024

While profiling hxb we came across an unrelated problem with typing passes. Delays for those are currently managed in a big (int * (unit -> unit)) list where the integer corresponds to the typing pass. Adding entries browses the list until the priority is equal (or larger for delay_late) and then inserts the new element. Flushing a given pass removes elements until the priority is lower.

The problem with the addition case is that it has linear cost over the number of entries. For example, any addition of the lowest priority PFinal will scroll past all current entries to insert it. This gets even worse because delay isn't tail-recursive, which leads to really deep call stacks.

This PR changes it so that we maintain an array of lists, where the array indices correspond to the priorities. Insertion then just has to index into the array and append to the list, while flushing checks all arrays in order until it finds one with a non-empty list, and then recurses after executing its task.

Importantly, flushing only ever handles the first element it finds for a given pass and then starts over. This is because any delay might insert a higher-priority delay which then has to be processed before the rest of our current priority list. In order to not do too many pointless array lookups, we remember the delayed_min_index, which is the lowest pass whose list might have entries.

While this is clearly the better approach in my head, it would be good to have some profiling data to support this.

@skial skial mentioned this pull request Jan 9, 2024
1 task
@Simn
Copy link
Member Author

Simn commented Jan 11, 2024

It's not very easy to get straightforward profiling data here, but I see in perf that the deep stack trace from delay is gone. Which of course I could have known without perf, because the loop itself is gone. With the delayed_min_index memoization, flush_pass should not suffer any performance penalty either. Will merge accordingly!

@Simn Simn merged commit 3af71d0 into development Jan 11, 2024
122 checks passed
@Simn Simn deleted the delay_buckets branch January 11, 2024 09:17
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
* [typer] bucket delay lists by typer pass

* track minimum index to avoid some loopings
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.

None yet

1 participant