Status Update
Comments
ra...@google.com <ra...@google.com>
ap...@google.com <ap...@google.com> #2
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
cc...@google.com <cc...@google.com> #3
ra...@google.com <ra...@google.com> #4
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
cc...@google.com <cc...@google.com> #5
Method traces are < 10 M for activity startup cases (~10s CUJ).
I expect startup cases we have are pretty trivial. What about scrolling cases?
introduce a phase
I don't know if I like that as the default, for method tracing or for sampling in macrobench, since tests are usually so much longer than micro. Consider a 5 min scrolling test, where you're just not collecting profiling results for minutes on end while the developer waits.
Could always eventually make it an option like androidx.benchmark.profiling.primaryMeasurePhase.enable
ap...@google.com <ap...@google.com> #6
Branch: androidx-main
commit 6a2d90e57ebe9de586b19f195a386229bffb9dc1
Author: Chris Craik <ccraik@google.com>
Date: Wed Jun 14 11:12:30 2023
Single output code for micro/macro studio metric and trace output
Bug: 227205461
Bug: 286306579
Bug: 285912360
Test: ./gradlew bench:b-c:cC
Test: ./gradlew bench:b-m:cC
Test: Micro, macro, and BP tests show correctly in Studio
Relnote: "Fix warnings to always be printed in Benchmark output in
Studio, and workaround leading whitespaces from Benchmark output not
showing up in Studio"
Unify ide output codepath for all benchmark/bp/tracerule code.
This is a necessary prerequisite for outputting additional metrics
from microbench, such as counters or trace metrics, so that when
additional metrics are included, they'll automatically show up.
This is also a necessary prerequisite for linking profiling traces
from macro in Studio.
This also fixed a couple long standing issues:
* Fixes warnings only going to one of the Studio v1/v2 string pair.
* Works around Studio swallowing leading whitespace by left aligning
macro metric names
Change-Id: Ia61d0fdc52ca0de2ece777ea6dd76bc77876fcf2
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/BenchmarkStateTest.kt
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/InstrumentationResultsTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/BenchmarkState.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/InstrumentationResults.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/Profiler.kt
M benchmark/benchmark-junit4/src/main/java/androidx/benchmark/junit4/PerfettoTraceRule.kt
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/FileLinkingRule.kt
D benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/IdeSummaryStringTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/BaselineProfiles.kt
D benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/IdeSummaryString.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
ap...@google.com <ap...@google.com> #7
Branch: androidx-main
commit f3e4a27c3206c4f35f4c0d7e4fc872f9f6e48ec7
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Fri Jun 23 14:58:20 2023
Report method traces in the benchmark output.
Bug: 285912360
Test: Trivial Startup benchmarks.
Change-Id: I374c33576d54f04d9631fdd70f3d84f816af482a
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
an...@google.com <an...@google.com> #8
Chris suggested I share some feedback from my recent try to have method tracing for the macro bench run. I was running a compose TrivialListScrollBenchmark I added
android {
defaultConfig {
testInstrumentationRunnerArguments["androidx.benchmark.profiling.mode"] = 'MethodTracing'
}
}
Into gradle and made a run. Each iteration created two files:
- perfetto-trace which only contains the manually added traces (as if MethodTracing wasn't enabled)
- method.trace files. a file for the 0th iteration contains some trace which I can open in studio or
https://profiler.firefox.com , but I don't understand what part of the execution was tested. it shows a trace for a one Choreographer.doFrame where we composed a new item, but the whole run includes many frames for multiple flings and I am not sure what frame it chose. another interesting part is that method.trace files for each other iteration are just zero sized files Attaching those files
ap...@google.com <ap...@google.com> #9
Branch: androidx-main
commit b545c369ec7d8d6a019616c2dff67a4819c655c8
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Wed Aug 23 13:24:01 2023
Make method tracing in Macrobenchmark more resilient.
* Use `--streaming`. Otherwise method traces stop at ~ the 1 sec mark.
* Use `--start-profiler` only when the process is not already alive.
* Tracing is always stopped at an iteration boundary to avoid confusion.
* Only iterations that involve `cold` starts are now accompanied with a method trace and therefore we avoid `0 byte` file captures.
Test: Tested locally.
Bug:
Change-Id: I66670cc981f371359767588c0374b1a909972b6c
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkScopeTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
ra...@google.com <ra...@google.com> #10
I landed this change to make this more consistent.
--
In general, MethodTracing
when enabled captures the time between a cold start to the point where the iteration is complete.
We only turn on method tracing for iterations that involve cold starts. You will therefore not see a 1-1 mapping between the number of iterations and the number of method traces given you might be measuring HOT starts for e.g.
I fixed the issue where you were seeing 0
byte method trace files as well. Let me know if you have other questions.
cc...@google.com <cc...@google.com> #11
I'm going to do one more pass on the behavior here to make it match micro more closely for the final 1.2 version:
- One method tracing pass at the end, after measurements, with measurements disabled (may want to still throw if no metrics captured, though)
- Document behavior surprises for micro vs macro
- Starts in first startActivityAndWait for now
- Add warning to output about why missing in living process case
ap...@google.com <ap...@google.com> #12
Branch: androidx-main
commit 98cd86539257998d458a86d2b689e3911687a4fd
Author: Chris Craik <ccraik@google.com>
Date: Mon Sep 11 15:50:57 2023
Fix method tracing file labels/paths
Test: TrivialStartupBenchmark # w/ method tracing enabled
Bug: 285912360
Relnote: "(Macrobenchmark) Clarified method tracing label in Studio
display, and fixed method tracing filenames to be unique on
device/host, so they won't be overwritten when more than one benchmark
is run."
Method tracing still has some issues, such as only capturing one
iteration when process isn't killed each iteration, and not running as
a separate phase, but those are larger changes, for a later release.
Also, removes unused MethodTracing.kt
Change-Id: I08e65d3fe8ab77614b5a3cbc241fb7901ba57135
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkScopeTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
D benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MethodTracing.kt
cc...@google.com <cc...@google.com> #13
With the above fix, macrobench method tracing is in a good enough/experimental state to ship with 1.2, so punting further work to 1.3
Remaining issues / gaps between method tracing in micro vs macro:
- Method trace should be done in separate measurement phase, more like microbench
- Method tracing can only start for an iteration if process is launched cold, so only one occurs frequently, e.g. when measuring warm start (CL in
adds warning about this)#comment12 - Method trace not embedded into perfetto trace
ap...@google.com <ap...@google.com> #14
Branch: androidx-main
commit 7a0dc4138dd47a207c94dead3653e4db2c36b1d9
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Mon Nov 06 15:12:10 2023
Use a consistent naming scheme for generated method traces in Macrobenchmarks.
Test: Updated existing tests.
Bug: 285912360
Change-Id: I5367d4fad0ec62a761ed9272340638d0020cfd75
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkScopeTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
cc...@google.com <cc...@google.com>
bu...@google.com <bu...@google.com> #16
bu...@google.com <bu...@google.com>
ra...@google.com <ra...@google.com>
cc...@google.com <cc...@google.com> #17
Last thing here should be to not capture metrics (or at least label metrics separately) for method tracing iterations.
ra...@google.com <ra...@google.com>
ap...@google.com <ap...@google.com> #18
Branch: androidx-main
commit 2d1785e9837b97c6d64878413efb96d873895cd0
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Wed Apr 24 16:57:02 2024
Move method tracing to a separate phase.
* Method tracing now runs as a separate phase, without interfering with metrics.
Fixes: 285912360
Fixes: 336588271
Test: Existing unit tests.
Change-Id: If9a504a7bce7228840339b34294ba3fdf98aceeb
Relnote: Method tracing runs as a seperate phase during a Macrobenchmark, and it no longer affects measurements.
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
A benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkPhase.kt
pr...@google.com <pr...@google.com> #19
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.benchmark:benchmark-macro:1.3.0-alpha05
Description
Filing a bug to track (late) comments on macrobench tracing CL:https://android-review.git.corp.google.com/c/platform/frameworks/support/+/2594692
Can punt this out of 1.2, but put there for minimal fix of config options