Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: retain channel waiter condition variables
The optimization is sound. Retaining the initialized pthread cond+mutex with the per-goroutine cached chanWaiter and skipping re-init() on reuse is correct: the signal is unlocked when cached (z_chan.go:282) and re-locked on reuse (z_chan.go:234), the park()/finish() handshake still holds the lock through notification, the cache is strictly per-goroutine, the field-by-field reset currently covers every non-signal field, and the finish() comment reword is accurate.
Two items, flagged inline:
- [P1] Resource leak on the lifecycle-owned teardown path —
mexitdestroys the retained signal only for!ownedByLifecycle, but lifecycle-owned goroutines (main goroutine, foreign/host threads) tear down throughdestroyGing_tls.go, which never destroysgp.chanWaiter.signal. Seeproc_pthread.go. - [P3] Manual field reset is fragile against future struct fields — see
z_chan.go.
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
String equality benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
go-wyvern
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Retain the initialized native condition variable and mutex on the current G's cached ordinary channel waiter, building on the waiter reuse already present in the base branch. Completed waits clear operation references in place, and initialized native synchronization objects remain in place for reuse.
Runtime-owned workers destroy the retained signal in
mexit. Main and foreign-thread contexts destroy it indestroyGbefore their context root is released. A waiter that cannot enter the one-entry cache still destroys its signal immediately.Add Go and LLGo goroutine lifecycle benchmarks covering goroutine creation and serial dispatch through one persistent worker. The serial-dispatch workload reuses request and completion channels, so it exercises the blocking channel waiter path on LLGo and reports an end-to-end signal for the runtime change. The benchmark baseline artifact and pull request table include both workloads for batches of 1, 16, 128, and 1024.
Validation
go test ./benchmark/goroutine ./benchmark/baseline -count=1TestReusableWorkerBatches.serial-dispatchfor all four task counts.go fmt ./...,git diff --check, andgo vet -copylocks=false ./internal/buildcompleted. Fullgo vet ./...still reports existing fixture and copylocks warnings outside this change.The serial-dispatch benchmark measures channel synchronization together with worker scheduling and task execution. Lower task counts make waiter costs easier to observe, while larger batches include more task and scheduling work. A dedicated channel-waiter microbenchmark or profile remains useful when attributing the exact portion of the improvement.