Skip to content

Reduce the amount of goroutines used for builds#126

Merged
msujew merged 6 commits into
mainfrom
msujew/reduce-goroutines
Jul 20, 2026
Merged

Reduce the amount of goroutines used for builds#126
msujew merged 6 commits into
mainfrom
msujew/reduce-goroutines

Conversation

@msujew

@msujew msujew commented Jul 8, 2026

Copy link
Copy Markdown
Member

Massively reduces the amount of spawned goroutines required for a workspace build. I noticed that during large workspace builds, we spawn an unreasonable number of goroutines, which (a) drags down our memory profile and (b) slows down runtime performance when multithreading (but also when single-threading!).

This change adds a new parallel.ForEachIter (iter.Seq[T]) and parallel.ForEach ([]T) for efficient chunking of data for goroutines. This is now used for both the linking and general workspace build phase (essentially all steps where we use sync.WaitGroup), which results in a total speedup of roughly 40%, and a 20% memory usage decrease.

@msujew
msujew requested a review from spoenemann July 8, 2026 15:57

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 6271adf Previous: d803417 Ratio
BenchmarkWorkspaceCycle (typefox.dev/fastbelt/examples/statemachine) - MB/s 13.28 MB/s 5.21 MB/s 2.55

This comment was automatically generated by workflow using github-action-benchmark.

@msujew

msujew commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Ignore comment above by the git-bot. It doesn't understand that 11MB/s is better than 5MB/s 🙄

@Lotes Lotes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you adjust the performance check in that regard that improvements of the speed are not flagged?
On the other hand this can also be an indicator that important code was skipped. But actually then the tests should also fail...

I added some comments about the parallelization module.

Comment thread util/parallel/parallel.go Outdated
Comment thread util/parallel/parallel_test.go
Comment thread util/parallel/parallel.go Outdated
Comment thread util/parallel/parallel.go Outdated
Comment thread util/parallel/parallel.go

@Lotes Lotes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fine for me.

@sailingKieler sailingKieler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice improvement @msujew 👍
I have some remarks and suggestions, see below.

Comment thread ast.go Outdated
Comment thread workspace/builder.go
Comment thread util/parallel/parallel.go Outdated
@msujew

msujew commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

FYI I've also introduced a parallel.RunningAverage struct that helps us reduce effort while populating the References slice. It's reused in the lexer as well.

@sailingKieler sailingKieler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is basically fine for me, but I have some additional findings, see below.

Please update the PR title before merging 😉

Comment thread util/parallel/parallel.go Outdated
Comment thread util/parallel/parallel.go Outdated
Comment thread workspace/builder.go
Comment thread ast.go Outdated

@sailingKieler sailingKieler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, good to go from my point of view! 🚀

@sailingKieler

Copy link
Copy Markdown
Member

... don't forget to update the title/final commit msg 😉

@msujew
msujew merged commit b789ac2 into main Jul 20, 2026
5 checks passed
@msujew
msujew deleted the msujew/reduce-goroutines branch July 20, 2026 10:10
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.

3 participants