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

Wake up suspended scheduler threads if there is work for another one #3153

Merged
merged 1 commit into from
May 28, 2019

Conversation

dipinhora
Copy link
Contributor

Prior to this commit, suspended scheduler threads would not be woken
up even if there was more than one actor to be run in parallel.

This commit changes things so that if a scheduler thread has more
than one actor that has work to be done, it will wake up a suspended
scheduler thread to run the additional actor in parallel.

Resolves #3151.

Prior to this commit, suspended scheduler threads would not be woken
up even if there was more than one actor to be run in parallel.

This commit changes things so that if a scheduler thread has more
than one actor that has work to be done, it will wake up a suspended
scheduler thread to run the additional actor in parallel.

Resolves ponylang#3151.
@dipinhora
Copy link
Contributor Author

This PR technically does not resolve #3151 because the pathological example in that issue still exhibits the same behavior as without the changes in this PR. Pathological here means significant work being done in the create constructor instead of calling a behavior to do the actual work once the actor has been successfully constructed by create. Just as GC in Pony only works between behaviors, the scheduler scaling logic to determine if scheduler threads need to be resume relies work being done across behavior calls function correctly. The following examples should hopefully illustrate what I'm referring to.

If I run a slightly modified version of the pathological example (to increase the array size so I can reproduce the issue locally), htop shows only 2 scheduler threads in use out of a total of 8 available (before or after this PR):

use "collections"
actor Main

      new create(env:Env) =>
            var size:USize = 100 * 1024 * 1024
            var worker:USize = 100

            var arr:Array[I32] = Array[I32].init(0,size)

            for i in Range(0,worker) do
                  Worker(4096)
            end


actor Worker
      new create(size:USize) =>
            for i in Range(0,size) do
                  for j in Range (0, size) do
                        var current:USize = 0
                        for k in Range (0, size) do
                              current = current + (i * j* k)
                        end
                  end
            end

If I run the following less pathological variant, htop shows 7 scheduler threads in use out of a total of 8 available (after this PR; it shows 2 before):

use "collections"
actor Main

      new create(env:Env) =>
            var size:USize = 100 * 1024 * 1024
            var worker:USize = 100

            var arr:Array[I32] = Array[I32].init(0,size)

            for i in Range(0,worker) do
                  Worker(4096)
            end


actor Worker
      new create(size:USize) => do_work(size)

      be do_work(size:USize) =>
            for i in Range(0,size) do
                  for j in Range (0, size) do
                        var current:USize = 0
                        for k in Range (0, size) do
                              current = current + (i * j* k)
                        end
                  end
            end

If I run this last variant, htop shows all 8 scheduler threads in use out of a total of 8 available (after this PR; it shows 2 before):

use "collections"
actor Main

      new create(env:Env) =>
            var size:USize = 100 * 1024 * 1024
            var worker:USize = 100

            var arr:Array[I32] = Array[I32].init(0,size)

            for i in Range(0,worker) do
                  Worker(4096)
            end


actor Worker
      new create(size:USize) => do_work(size)

      be do_work(size:USize) =>
            for i in Range(0,size) do
                  do_sub_work(i, size)
            end

      be do_sub_work(i: USize, size:USize) =>
                  for j in Range (0, size) do
                        var current:USize = 0
                        for k in Range (0, size) do
                              current = current + (i * j* k)
                        end
                  end

Given the above examples, I believe this PR resolves the core issue and the pathological case shouldn't be a concern as it goes against Pony best practices.

@mfelsche
Copy link
Contributor

mfelsche commented May 15, 2019

Great PR!

This might be a good hint for the performance cheat sheet.

@aturley aturley added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label May 28, 2019
@SeanTAllen SeanTAllen merged commit 45ae31e into ponylang:master May 28, 2019
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.

Scheduler scaling issue
5 participants