Status Update
Comments
ro...@google.com <ro...@google.com> #2
Thanks for your detailed post.
However, benchmark build type is under configured: at least isProfileable is not set to true for existing build type, probably there's more.
We should set isProfileable = true
by default when overriding an existing benchmark build type.
This issue is not about some specific configuration flag, but the general approach of dealing with external configuration. As a developer adopting baseline profiles, it seems extremely risky to me using a custom configuration due to how it's applied under the hood and the fact it may break default configuration.
I agree that is not great but this is a little tricky to do. For custom baseline profile build types we override all the properties. For benchmark I left it open to configure but it's mostly about these 2 properties:
isMinifyEnabled
isShrinkResources
I don't have a way to see if the user is setting them before overriding, so for this reason, I'd prefer not to. I agree with you that some other properties could be set by default to make this easier, i.e.:
isJniDebuggable = false
isDebuggable = false
isProfileable = true
The reason why I mentioned the release signing config in the beginning is because I want to use debug signing config.
In the specific of your issue, i.e. using a debug certificate can you override the benchmark and baseline profile setting? You should be able to do something like:
android {
buildTypes {
release { ... }
debug { ... }
benchmarkRelease {
...
signingConfig signingConfigs.debug
}
nonMinifiedRelease {
...
signingConfig signingConfigs.debug
}
}
}
cc...@google.com <cc...@google.com> #3
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Albano <
Link:
Added override for debuggable and profileable for benchmark builds in bpgp
Expand for full commit details
Added override for debuggable and profileable for benchmark builds in bpgp
Test: ./gradlew :benchmark:benchmark-baseline-profile-gradle-plugin:test
Bug: 369213505
Relnote: "isProfileable is always overridden in benchmark builds,
and isDebuggable is also now always overridden in both benchmark and
nonMinified (baseline profile capture) builds."
Change-Id: I487fa71083921682173f04fcbb477be5baf165f8
Files:
- M
benchmark/baseline-profile-gradle-plugin/src/main/kotlin/androidx/baselineprofile/gradle/apptarget/BaselineProfileAppTargetPlugin.kt
- M
benchmark/baseline-profile-gradle-plugin/src/test/kotlin/androidx/baselineprofile/gradle/apptarget/BaselineProfileAppTargetPluginTest.kt
Hash: 1906bbe52ba7ccb9ca0e1c1d6de33e7c91b5c6f0
Date: Fri Oct 11 10:07:06 2024
ma...@google.com <ma...@google.com> #4
I've landed a change that will set the following properties also when the benchmark build type already exists:
isJniDebuggable = false
isDebuggable = false
isProfileable = true
As well as the following for agp 8.0:
isDebuggable = false
I'm going ahead and closing this - if you've further questions please answer here and will reopen. Thanks.
ro...@google.com <ro...@google.com> #5
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.benchmark:benchmark-baseline-profile-gradle-plugin:1.4.0-alpha04
ap...@google.com <ap...@google.com> #6
Since release signing config is used by default instead of debug
is not mentioned in release notes - maybe this is a bug?
Cause the last time I found it mentioned in the release notes was in version 1.1
signingConfig.debug is used as the default signing config (
) b/153583269
So, if the switch to the release one indeed happened - maybe it's an issue?
ap...@google.com <ap...@google.com> #8
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> #9
Branch: androidx-main
commit f32334a2f282467d49820319b94948ad80ac3768
Author: Chris Craik <ccraik@google.com>
Date: Wed Jun 21 15:20:13 2023
Add CpuEventCounters to microbenchmarks behind flag, root required
Test: CpuEventCounterTest
Test: TrivialKotlinBenchmark
Test: PerfettoOverheadBenchmark
Test: SynchronizedBenchmark
Test: CpuEventCounterBenchmark
Test: LazyListScrollingBenchmark
Bug: 286306579
Also improves warmup to use repeat duration directly, instead of
bypassing pause during warmup, since this was necessary to avoid
`measureRepeated { runWithTimingDisabled {} }` from taking an
extremely long time. What would happen is that the loop would take
e.g. 10ns during warmup, and say 2000ns during the actual run, since
starting/stopping counters is non-trivial. This would mean that
warmupManager would overshoot the target duration by 20x, since it
wasn't accounting for the true cost of pausing/resuming. Now, warmup
much more closely matches timing, which also helps us avoid hitting
cold code suddenly.
Set flags like the following:
```
testInstrumentationRunnerArguments["androidx.benchmark.cpuEventCounters.enable"]= 'true'
testInstrumentationRunnerArguments["androidx.benchmark.cpuEventCounters.timingPhaseMask"]= 'Instructions,CpuCycles'
```
Example output:
PerfettoOverheadBenchmark.traceBeginEnd
timeNs min 1,495.7, median 1,511.2, max 1,620.2
Instructions min 3,845.0, median 3,845.0, max 4,007.1
CpuCycles min 1,635.5, median 1,642.2, max 1,817.4
allocationCount min 0.0, median 0.0, max 0.0
PerfettoSdkOverheadBenchmark.traceBeginEnd_perfettoSdkTrace
timeNs min 580.8, median 585.4, max 653.6
Instructions min 3,830.6, median 3,959.1, max 3,969.1
CpuCycles min 1,642.3, median 1,650.5, max 1,668.8
allocationCount min 0.0, median 0.0, max 0.0
LazyListScrollingBenchmark.scrollProgrammatically_newItemComposed[LazyColumn]
timeNs min 273,686.5, median 275,024.7, max 364,566.3
Instructions min 642,749.5, median 642,837.1, max 683,841.6
CpuCycles min 706,229.0, median 709,797.1, max 808,859.2
allocationCount min 490.3, median 490.3, max 490.4
Change-Id: Ic938690476296899f058271ed56dfc3ee95aa2cf
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/BenchmarkStateConfigTest.kt
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/CpuEventCounterTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/Arguments.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/BenchmarkState.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/CpuEventCounter.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricCapture.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricsContainer.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MicrobenchmarkPhase.kt
D benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/CpuCounterBenchmark.kt
A benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/CpuEventCounterBenchmark.kt
ma...@google.com <ma...@google.com>
na...@google.com <na...@google.com> #10
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.benchmark:benchmark-common:1.2.0-beta01
androidx.benchmark:benchmark-junit4:1.2.0-beta01
androidx.benchmark:benchmark-macro:1.2.0-beta01
cc...@google.com <cc...@google.com>
cc...@google.com <cc...@google.com> #11
Over to Marcello to enable this in CI
ap...@google.com <ap...@google.com> #12
Branch: androidx-main
commit 3383ab2cbccb3015549fb91a945678310e75f625
Author: Marcello Albano <maralb@google.com>
Date: Tue Oct 17 14:51:39 2023
Fixed tests and added measure api to CpuEventCounter
Bug: 286306579
Test: existing
Change-Id: Ia2bf481ae6e7e2f95e5b3727bb6686eafb3ba5a6
M buildSrc-tests/src/test/java/androidx/build/testConfiguration/AndroidTestConfigBuilderTest.kt
M buildSrc/private/src/main/kotlin/androidx/build/testConfiguration/AndroidTestConfigBuilder.kt
ap...@google.com <ap...@google.com> #13
Branch: androidx-main
commit e52411023650ad95306ddf4db1dd3b757cb39348
Author: Marcello Albano <maralb@google.com>
Date: Tue Oct 24 10:13:35 2023
Fixed name in cpu counter events
To be consistent we want all the cpu counter event names to be camel
case.
Bug: 286306579
Test: existing
Change-Id: Id6642294c3884047166a28b738ee53f9444080a6
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricNameUtils.kt
ma...@google.com <ma...@google.com> #15
CI integration had to be reverted because of
It seems that all the benchmarks are slowing down because of enabling the cpu counters.
cc...@google.com <cc...@google.com>
ap...@google.com <ap...@google.com> #16
Branch: androidx-main
commit 5de09686debf78730da51df37c78f91e77d3dbc5
Author: Chris Craik <ccraik@google.com>
Date: Fri Jul 26 15:30:55 2024
Fix pause/resume ordering to preserve metric priority
Test: MetricsContainerTest#validatePriorityOrder
Bug: 286306579
Bug: 307445225
Relnote: "Fix resumeTiming/runWithTimingDisabled to respect metric
priority order, and significantly reduce impact of lower priority
metric pause/resume on higher priority metric results. For example, if
using cpu perf counters via `cpuEventCounter.enable` instrumentation
argument, timeNs is no longer significantly reduced when pause/resume
occur."
Drastically reduces impact of perf event counters on timeNs measurements
-- Baseline, perf counters disabled -- (here and below running on mokey 32 bit, jit disabled)
136,714 ns 11 allocs Trace LazyListScrollingBenchmark.drawAfterScroll_newItemComposed[LazyColumn]
-- Before Change, perf counters enabled --
LazyListScrollingBenchmark.drawAfterScroll_newItemComposed[LazyColumn]
timeNs min 172,242.6, median 185,443.2, max 216,688.5
Instructions min 87,728.2, median 88,600.8, max 93,946.2
CpuCycles min 145,951.4, median 151,045.5, max 174,344.5
allocationCount min 11.0, median 11.0, max 11.0
-- After Change, perf counters enabled --
LazyListScrollingBenchmark.drawAfterScroll_newItemComposed[LazyColumn]
timeNs min 140,890.3, median 149,411.4, max 199,267.2
Instructions min 87,940.5, median 89,342.1, max 95,317.8
CpuCycles min 146,792.6, median 152,793.3, max 180,540.3
allocationCount min 11.0, median 11.0, max 11.0
Change-Id: I39c2eb911129927972740d074ee8f2adca7bda1a
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/MetricsContainerTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricsContainer.kt
ap...@google.com <ap...@google.com> #17
Branch: androidx-main
commit 06edd596b9ced3114159059ba176c7ec173fc51d
Author: Chris Craik <ccraik@google.com>
Date: Fri Jul 26 15:46:39 2024
Fix corrupted counter outputs by fixing flag caching
Bug: 286306579
Test: MetricCaptureTest#cpuEventCounterCapture_multi
Relnote: "Fix `androidx.benchmark.cpuEventCounter` producing corrupt
values for non-Instruction events"
Previously, running multiple benchmarks in a row with cpu event
counters would oly capture Instructions after the first benchmark,
with garbage values for the rest. This was caused by improper caching
of flags above the native layer. Moved the cache to a lower level to
avoid this problem.
Also moves the DEFAULT_METRICS definition inline to avoid a class init
ordering problem in tests, which further avoids this problem by not
caching a used CounterCapture instance across multiple tests
Change-Id: I7386abe3494b2b11a2447fa85a4109613857c28c
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/MetricCaptureTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/BenchmarkState.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/CpuEventCounter.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricCapture.kt
cc...@google.com <cc...@google.com> #18
Remaining work (moved to 1.4 hotlist):
- Renaming counters to be
camelCase
- Fixing API 28 permission errors, which failed in ABTD
- Enabling for AndroidX in CI
- Experimental public MetricCapture API (with simplified lifecycle)
ap...@google.com <ap...@google.com> #19
Branch: androidx-main
commit 0df5a6108af99875485daacf7c7c82680fe2eeaa
Author: Chris Craik <ccraik@google.com>
Date: Wed Jul 31 14:11:24 2024
Use camelCase for cpu event counter metrics
Bug: 286306579
Test: cpuEventCounterCapture_outputName
This was already done for instrumentation arg output, this change
makes it consistent across Studio and JSON as well.
Also switches to fixed Locale so test device locale doesn't affect
test output formatting.
Change-Id: I9d74f0327536866b54605b2ed2f61d4c04c468f3
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/MetricCaptureTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/CpuEventCounter.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricCapture.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MetricNameUtils.kt
ap...@google.com <ap...@google.com> #20
Branch: androidx-main
commit 2550048909d02279f8a0e759e7c21dabab35f93a
Author: Chris Craik <ccraik@google.com>
Date: Thu Aug 01 15:48:29 2024
Change perf event enable logic to run on API 23+
Test: Tested on API 23, 27, 28, all of which needed this change (Unable to get access to a lower API physical device at the moment)
Bug: 286306579
Bug: 357101113
Change-Id: I283018817556b18cbedb77e11a3c95916e3c7ed3
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/CpuEventCounterTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/CpuEventCounter.kt
ap...@google.com <ap...@google.com> #21
Branch: androidx-main
commit 992cb25c28026d5f259edb7459f6d7c6be43ce34
Author: Chris Craik <ccraik@google.com>
Date: Fri Jul 26 15:59:45 2024
Reapply "Fixed tests and added measure api to CpuEventCounter"
Bug: 286306579
Bug: 307445225
Test: AndroidTestConfigBuilderTest
Reenables microbenchmark cpu counters in CI, now that intrusiveness
has been addressed.
This reverts commit 698248c877020adb675fd30becc55551e8ffc281.
Change-Id: I024f82907805d54eaf98c9233c1766e79a1b5dfd
M buildSrc-tests/src/test/java/androidx/build/testConfiguration/AndroidTestConfigBuilderTest.kt
M buildSrc/private/src/main/kotlin/androidx/build/testConfiguration/AndroidTestConfigBuilder.kt
cc...@google.com <cc...@google.com> #22
Punting the last bit to 1.5:
- Experimental public MetricCapture API (with simplified lifecycle)
Description
Planning to take code from Filament's Profiler.