Skip to content

Use WorkStealingDispatcher in runtime, behind a flag. #1365

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 23, 2025
Merged

Use WorkStealingDispatcher in runtime, behind a flag. #1365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad7bcf6
Use WorkStealingDispatcher in runtime, behind a flag.
zach-klippenstein Jul 9, 2025
d9bbcff
Update WSD Use, Further Tests, Add to CI
steve-the-edwards Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions .github/workflows/kotlin.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'
Copy link
Contributor

@steve-the-edwards steve-the-edwards Jul 23, 2025

Choose a reason for hiding this comment

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

This was just a bug with the reporting paths that I found while looking at this.

Copy link
Collaborator

@rjrjr rjrjr Jul 23, 2025

Choose a reason for hiding this comment

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

What is report_paths?

Copy link
Contributor

@steve-the-edwards steve-the-edwards Jul 23, 2025

Choose a reason for hiding this comment

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

what files it tries to pick up test results from to show in the PR directly. (this was just not working before).


jvm-conflate-runtime-test:
name: CSR JVM Tests
Expand All @@ -260,7 +260,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-stateChange-runtime-test:
name: SCO JVM Tests
Expand All @@ -281,7 +281,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-stable-handlers-test:
name: SEH JVM Tests
Expand All @@ -302,7 +302,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-partial-runtime-test:
name: PTR JVM Tests
Expand All @@ -323,7 +323,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-conflate-stateChange-runtime-test:
name: SCO, CSR JVM Tests
Expand All @@ -344,7 +344,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-conflate-partial-runtime-test:
name: CSR, PTR JVM Tests
Expand All @@ -365,7 +365,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-conflate-drainExclusive-runtime-test:
name: CSR, DEA JVM Tests
Expand All @@ -386,7 +386,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-stateChange-drainExclusive-runtime-test:
name: SCO, DEA JVM Tests
Expand All @@ -407,7 +407,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-partial-drainExclusive-runtime-test:
name: PTR, DEA JVM Tests
Expand All @@ -428,7 +428,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-conflate-stateChange-drainExclusive-runtime-test:
name: SCO, CSR, DEA JVM Tests
Expand All @@ -449,7 +449,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-conflate-partial-drainExclusive-runtime-test:
name: CSR, PTR, DEA JVM Tests
Expand All @@ -470,7 +470,28 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

jvm-all-runtime-test:
name: ALL Optimizations JVM Tests
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

- name: Check with Gradle
uses: ./.github/actions/gradle-task
with:
task: jvmTest --continue -Pworkflow.runtime=all
Copy link
Contributor

@steve-the-edwards steve-the-edwards Jul 23, 2025

Choose a reason for hiding this comment

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

I don't test the work stealing dispatcher combinations separately, just adding the 'all' config here for the jvm tests.

Copy link
Collaborator

@rjrjr rjrjr Jul 23, 2025

Choose a reason for hiding this comment

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

Which should be fine b/c we plan to ship that way?

Copy link
Contributor

@steve-the-edwards steve-the-edwards Jul 23, 2025

Choose a reason for hiding this comment

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

'eventually', yes

Copy link
Contributor

@steve-the-edwards steve-the-edwards Jul 23, 2025

Choose a reason for hiding this comment

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

but good point that we should make sure each combination we roll out with has been tested here.

restore-cache-key: main-build-artifacts

# Report as GitHub Pull Request Check.
- name: Publish Test Report
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/jvmTest/TEST-*.xml'

ios-tests:
name: iOS Tests
Expand All @@ -491,7 +512,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/iosX64Test/TEST-*.xml'

js-tests:
name: JS Tests
Expand All @@ -513,7 +534,7 @@ jobs:
uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4
if: always() # always run even if the previous step fails
with:
report_paths: '**/build/test-results/test/TEST-*.xml'
report_paths: '**/build/test-results/jsTest/TEST-*.xml'

performance-tests:
name: Performance tests
Expand Down Expand Up @@ -578,7 +599,7 @@ jobs:
### <start-connected-check-shards>
shardNum: [ 1, 2, 3 ]
### <end-connected-check-shards>
runtime: [ conflate, stateChange, drainExclusive, conflate-stateChange, partial, conflate-partial, stable, conflate-drainExclusive, stateChange-drainExclusive, partial-drainExclusive, conflate-partial-drainExclusive ]
runtime: [ conflate, stateChange, drainExclusive, conflate-stateChange, partial, conflate-partial, stable, conflate-drainExclusive, stateChange-drainExclusive, partial-drainExclusive, conflate-partial-drainExclusive, all ]
Copy link
Contributor

@steve-the-edwards steve-the-edwards Jul 23, 2025

Choose a reason for hiding this comment

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

Add 'all' the runtime test shard.

steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
Expand Down Expand Up @@ -618,6 +639,7 @@ jobs:
- jvm-partial-drainExclusive-runtime-test
- jvm-conflate-stateChange-drainExclusive-runtime-test
- jvm-conflate-partial-drainExclusive-runtime-test
- jvm-all-runtime-test
- ktlint
- performance-tests
- runtime-instrumentation-tests
Expand Down
8 changes: 8 additions & 0 deletions ...g/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS
import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING
import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES
import com.squareup.workflow1.RuntimeConfigOptions.STABLE_EVENT_HANDLERS
import com.squareup.workflow1.RuntimeConfigOptions.WORK_STEALING_DISPATCHER
import com.squareup.workflow1.WorkflowExperimentalRuntime

public class AndroidRuntimeConfigTools {
Expand Down Expand Up @@ -38,6 +39,11 @@ public class AndroidRuntimeConfigTools {
* - `drainExclusive` Enables draining exclusive actions. If we have more actions to process
* that are queued on nodes not affected by the last action application, then we will
* continue to process those actions before another render pass.
*
* - `stealingDispatcher` Enables turning on the [WorkStealingDispatcher] to try and drain
* available tasks.
*
* - `all` Enables all optimizations.
*/
@WorkflowExperimentalRuntime
public fun getAppWorkflowRuntimeConfig(): RuntimeConfig {
Expand All @@ -54,6 +60,8 @@ public class AndroidRuntimeConfigTools {
"partial" -> config.addAll(setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING))
"stable" -> config.add(STABLE_EVENT_HANDLERS)
"drainExclusive" -> config.add(DRAIN_EXCLUSIVE_ACTIONS)
"stealingDispatcher" -> config.add(WORK_STEALING_DISPATCHER)
"all" -> config.addAll(RuntimeConfigOptions.ALL)
else -> throw IllegalArgumentException("Unrecognized runtime config option \"$it\"")
}
}
Expand Down
8 changes: 8 additions & 0 deletions ...onfig/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS
import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING
import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES
import com.squareup.workflow1.RuntimeConfigOptions.STABLE_EVENT_HANDLERS
import com.squareup.workflow1.RuntimeConfigOptions.WORK_STEALING_DISPATCHER
import com.squareup.workflow1.WorkflowExperimentalRuntime

public class JvmTestRuntimeConfigTools {
Expand Down Expand Up @@ -40,6 +41,11 @@ public class JvmTestRuntimeConfigTools {
* - `drainExclusive` Enables draining exclusive actions. If we have more actions to process
* that are queued on nodes not affected by the last action application, then we will
* continue to process those actions before another render pass.
*
* - `stealingDispatcher` Enables turning on the [WorkStealingDispatcher] to try and drain
* available tasks.
*
* - `all` Enables all optimizations.
*/
@OptIn(WorkflowExperimentalRuntime::class)
public fun getTestRuntimeConfig(): RuntimeConfig {
Expand All @@ -56,6 +62,8 @@ public class JvmTestRuntimeConfigTools {
"partial" -> config.addAll(setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING))
"stable" -> config.add(STABLE_EVENT_HANDLERS)
"drainExclusive" -> config.add(DRAIN_EXCLUSIVE_ACTIONS)
"stealingDispatcher" -> config.add(WORK_STEALING_DISPATCHER)
"all" -> config.addAll(RuntimeConfigOptions.ALL)
else -> throw IllegalArgumentException("Unrecognized runtime config option \"$it\"")
}
}
Expand Down
Loading
Loading