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

add buffer to promise like channel #2011

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vladopajic
Copy link
Contributor

PR Description

add buffer to promise like channel so main goroutine loop doesn't block

Which issue(s) this PR fixes

no issue, just performance improvement

Notes to the Reviewer

hey @mattdurham, I was reviewing usage of go-actor and just noticed this so I wanted to share it with you.

in DoWork caller is being notified after config gets updated. because this channel is not buffered this goroutine will get blocked because caller is also blocked at this point (it's waiting on response from done channel). main loop will remain blocked until goroutine that made this request is resumed again (which can take time theoretically speaking).

probably the best approach when implementing logic of DoWork (aka workers) is to never have it's code blocked unless it is blocked on reading data from inbound mailboxes - in other words blocked because it's waiting on work.

in this case, this can be easily achieved by making done, promise like channel, to have buffer of size 1. this will ensure that DoWork can write to it without waiting on this goroutine to be ready.


defer close(done) was removed because it is unnecessary, after data transmission, this channel is no longer used so no one will care if this channel is closed or not - eventually it will be garbage collected.

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@vladopajic vladopajic requested a review from a team as a code owner October 31, 2024 14:09
@mattdurham
Copy link
Collaborator

Since your here :) having an issue on where I don't know the number of callers, so if I close call and still have work coming in I get a panic which is not ideal. My idea is for actors that behave as a channel to not call close and not set the panic send handler. Thoughts?

@vladopajic
Copy link
Contributor Author

You mean mailboxes? To me it sound like you are using mailbox and sometimes you stop mailbox but there is more work coming in? If this is true then just just have to additional logic that will not send data to mailbox is something is sopped.

Or there is different issue, were you are stopping in different order. Inbound routine, that sends work, should be stopped first, before mailbox.

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.

2 participants