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

Make TCPConnection yield on writes to not hog cpu #3176

Merged
merged 2 commits into from
Jun 9, 2019

Conversation

dipinhora
Copy link
Contributor

Port over of the following PR from wallaroo:

WallarooLabs/wally#2565

This commit adds a _write_again to the TCPConnection actor
and modifies the _pending_writes function to yield by calling
_write_again after having sent at least _max_size bytes.
This will allow other actors and GC to run when a large amount
of data has been queued up to be sent.

Port over of the following PR from wallaroo:

WallarooLabs/wally#2565

This commit adds a `_write_again` to the `TCPConnection` actor
and modifies the `_pending_writes` function to yield by calling
`_write_again` after having sent at least `_max_size` bytes.
This will allow other actors and GC to run when a large amount
of data has been queued up to be sent.
@SeanTAllen
Copy link
Member

Can you add a CHANGELOG entry for this?

dipinhora added a commit to dipinhora/ponyc that referenced this pull request Jun 9, 2019
@dipinhora
Copy link
Contributor Author

@SeanTAllen changelog entry added. But it seems that my adding [skip ci] to it might not be ideal because it prevents the travis/circleci/appveyor tests from running.

@SeanTAllen
Copy link
Member

@dipinhora yeah, you'll need to rewrite to not have the [skip ci]

@dipinhora
Copy link
Contributor Author

@SeanTAllen [skip ci] issue fixed via force push. However, it seems appveyor is not running for it. Any ideas?

@SeanTAllen SeanTAllen merged commit 302dccf into ponylang:master Jun 9, 2019
patches11 pushed a commit to patches11/ponyc that referenced this pull request Jun 29, 2019
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