-
Notifications
You must be signed in to change notification settings - Fork 101
Add Baseline and Perf Tests to Perf Poetry #719
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
Conversation
2f01386
to
ec8d5f6
Compare
workflow-runtime/src/main/java/com/squareup/workflow1/internal/WorkflowRunner.kt
Outdated
Show resolved
Hide resolved
workflow-runtime/src/main/java/com/squareup/workflow1/internal/WorkflowNode.kt
Outdated
Show resolved
Hide resolved
ec8d5f6
to
0f29743
Compare
Still more work to do to refine these and establish the best scenarios, but this is a good basis and gets us setup in the application 'complex-poetry' where we should focus our performance testing in. |
workflow-runtime/src/main/java/com/squareup/workflow1/internal/WorkflowNode.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Rework to use
WorkflowInterceptor
56d4508
to
17ed0c5
Compare
...n/java/com/squareup/benchmarks/performance/poetry/complex/benchmark/ComplexPoetryResults.txt
Outdated
Show resolved
Hide resolved
...c/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoetryComponent.kt
Outdated
Show resolved
Hide resolved
...enchmark/src/main/java/com/squareup/sample/dungeon/benchmark/DungeonGatherBaselineProfile.kt
Show resolved
Hide resolved
benchmarks/performance-poetry/complex-benchmark/build.gradle.kts
Outdated
Show resolved
Hide resolved
} | ||
|
||
// kotlin_builtins duplication between 1.5.20 and 1.6.10? | ||
packagingOptions.exclude("**/*.kotlin_*") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
} | ||
|
||
dependencies { | ||
implementation(project(":benchmarks:performance-poetry:complex-poetry")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so what's a complex benchmark? What's a not complex one? Are there more interesting words we can use to describe the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless... this is about "ComplexPoetry". Why couldn't that have been just Poetry?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol. This is a 'Complex' version of the Poetry app.
benchmarkRule.measureRepeated( | ||
packageName = PACKAGE_NAME, | ||
metrics = traceMetricsList, | ||
iterations = 20, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure 20 is enough. What's the distribution looking like?
...n/java/com/squareup/benchmarks/performance/poetry/complex/benchmark/ComplexPoetryResults.txt
Outdated
Show resolved
Hide resolved
...n/java/com/squareup/benchmarks/performance/poetry/complex/benchmark/ComplexPoetryResults.txt
Outdated
Show resolved
Hide resolved
...c/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoetryComponent.kt
Outdated
Show resolved
Hide resolved
...c/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoetryComponent.kt
Outdated
Show resolved
Hide resolved
...c/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoetryComponent.kt
Outdated
Show resolved
Hide resolved
|
77bbe94
to
e6e0634
Compare
e6e0634
to
a6fcd2b
Compare
benchmarks/README.md
Outdated
@@ -1,22 +1,27 @@ | |||
# benchmarks | |||
|
|||
This module contains benchmarks. Used to measure and help improve Workflow performance. These tests | |||
should be run on physical devices. | |||
should be run on physical devices, or the closest approximation to physical devices you can get. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the deterministic tests, that's shouldn't be a requirement anymore, right?
complexityDelay = 100L | ||
), | ||
expectedPasses = 61, | ||
expectedRatio = 0.11891891891891893 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if you'd want to store the ratio as 2 numbers instead? idk feels weird to store a truncated number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, if you stored 2 numbers, you could then assert against each, and report whether there are less things fresh or more overall things.
a6fcd2b
to
abbea36
Compare
Use WorkflowInterceptor to count Render Passes and determine ratio of 'fresh' renderings.
abbea36
to
dac8999
Compare
Use
WorkflowInterceptor
to trace each render pass as well as particular nodes within it.We use a "Fresh Rendering Ratio" which is the # of render passes for a node that had a different RenderState / the total # of render passes.