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

Fix performance related to dynamic scheduler scaling #2751

Merged
merged 1 commit into from
Jun 6, 2018

Conversation

dipinhora
Copy link
Contributor

As part of performance testing Wallaroo using multiple workers,
@JONBRWN discovered a regression in both throughput and latency.
He tracked the issue down the commit that re-enabled dynamic
scheduler scaling (fc80968).
NOTE: This performance issue did not exist for singler worker
runs of Wallaroo.

Some head scratching and testing led to the current commit to
resolve the multi-worker performance issue. My best guess is that
before this change the steal loop was dependent on a memory
access to determine if dynamic scheduler scaling needed to
suspend a thread or not as its initial check. This would lead to
somewhat erratic behavior where some times the steal loop would
take long while other times it wouldn't depending on how long the
memory load took. This had a follow-on impact on actor execution
because of ASIO messages because they wouldn't be picked up off
of the queue for work as quickly as they could be due to the
extra memory accesses.

This commit changes the ordering of some operations to ensure
that there is more consistent memory accesses for the loop
resulting in more consistent actor actor execution for ASIO
messages resolving the multi-worker performance issue that
@JONBRWN discovered.

As part of performance testing Wallaroo using multiple workers,
@JONBRWN discovered a regression in both throughput and latency.
He tracked the issue down the commit that re-enabled dynamic
scheduler scaling (fc80968).
NOTE: This performance issue did not exist for singler worker
runs of Wallaroo.

Some head scratching and testing led to the current commit to
resolve the multi-worker performance issue. My best guess is that
before this change the `steal` loop was dependent on a memory
access to determine if dynamic scheduler scaling needed to
suspend a thread or not as its initial check. This would lead to
somewhat erratic behavior where some times the `steal` loop would
take long while other times it wouldn't depending on how long the
memory load took. This had a follow-on impact on actor execution
because of ASIO messages because they wouldn't be picked up off
of the queue for work as quickly as they could be due to the
extra memory accesses.

This commit changes the ordering of some operations to ensure
that there is more consistent memory accesses for the loop
resulting in more consistent actor actor execution for ASIO
messages resolving the multi-worker performance issue that
@JONBRWN discovered.
@SeanTAllen SeanTAllen changed the title Fix multi-worker performance related to dynamic scheduler scaling Fix performance related to dynamic scheduler scaling Jun 6, 2018
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Jun 6, 2018
@SeanTAllen SeanTAllen merged commit 5e35604 into ponylang:master Jun 6, 2018
ponylang-main added a commit that referenced this pull request Jun 6, 2018
@SeanTAllen SeanTAllen mentioned this pull request Jun 6, 2018
dipinhora pushed a commit to dipinhora/ponyc that referenced this pull request Jun 7, 2018
dipinhora pushed a commit to dipinhora/ponyc that referenced this pull request Jun 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants