Skip to content

RFC 0003: WaitGroup#3

Merged
straight-shoota merged 9 commits intomainfrom
rfc-wait-group
Apr 2, 2024
Merged

RFC 0003: WaitGroup#3
straight-shoota merged 9 commits intomainfrom
rfc-wait-group

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Feb 6, 2024

This PR has been merged. The RFC text is available at: https://github.com/crystal-lang/rfcs/blob/main/text/0003-wait-group.md


Provide a mechanism to wait on the execution of a set of operations distributed to a set of fibers.

Preview: https://github.com/crystal-lang/rfcs/blob/rfc-wait-group/text/0003-wait-group.md

@ysbaddaden ysbaddaden self-assigned this Feb 6, 2024
@ysbaddaden ysbaddaden changed the title rfc-wait-group RFC 0003: WaitGroup Feb 6, 2024
Comment on lines 117 to 120
What should happen when the counter reached zero? The `#add` and `#done` shall raise a RuntimeError exception, but what about waiting fibers? They might be stuck forever.

- Should the application abort (aka panic)?
- Should the waiting fibers be resumed _and_ also raise a RuntimeError exception?
Copy link
Member

Choose a reason for hiding this comment

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

What do the prior art implementations do in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • Java only checks the counter at initialization (can't add dynamically), then countDown() in OpenJDK is clamped to 0.. which overlooks the issue 🤷
  • Go panics when Add would decrement below 0 (similar to raising an exception), and leaves goroutines waiting (I feel like they won't be resumed if the panic is recovered).

Copy link
Member

Choose a reason for hiding this comment

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

My take on this: The intention of a wait group is to block waiting fibers until the counter reaches zero.
I think it would be reasonable to still resume the waiting fibers if it overshoots into the negative. It still means we've reached zero on the way...
Of course this is an invalid state, and #done/#add should raise on going negative.
And the counter should probably reset to 0 as to recover from this state.
Otherwise we would need all further calls to #wait raise immediately.

@straight-shoota
Copy link
Member

Related discussion on the forum: https://forum.crystal-lang.org/t/waitgroup-implementation/4293

@crysbot
Copy link

crysbot commented Feb 8, 2024

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/rfc-0003-waitgroup/6553/1

@mgomes
Copy link

mgomes commented Feb 11, 2024

Really happy to see this! It feels very Go-centric as is. I think having to manually add fiber counts and decrement via wg.done are what make it feel that way.

@RX14's suggested helper method would help a lot. Could the API used by https://github.com/GrottoPress/pond be adapted to be as efficient as this implementation?

It would look like this:

def sliced_operation(wg, i)
  32.times do |j|
    wg << spawn do
      sub_sliced_operation(i, j)
    end
  end
end

wg = WaitGroup.new

16.times do |i|
  wg << spawn do
    prepare_slice(i)
    sliced_operation(wg, i)
  end
end

wg.wait

@ysbaddaden
Copy link
Collaborator Author

@mgomes Pond has an interesting approach. It's closer to nurseries in structured concurrency (keep a list of child fibers) than a mere counter though, and I feel like it would and up duplicating what such nurseries would do.

Looking at your example, I feel that the #spawn(&) helper would do just the same, and I prefer how abstract it is over #<<(fiber).

@crysbot
Copy link

crysbot commented Feb 12, 2024

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/waitgroup-implementation/4293/7

@ysbaddaden
Copy link
Collaborator Author

I resolved the unresolved questions as discussed above.

@straight-shoota straight-shoota merged commit a2d6461 into main Apr 2, 2024
@straight-shoota straight-shoota deleted the rfc-wait-group branch April 2, 2024 12:32
straight-shoota added a commit to nobodywasishere/crystal-rfcs that referenced this pull request Feb 25, 2025
straight-shoota added a commit to meatball133/rfcs that referenced this pull request Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants