CI: performance impact measurements #2171

vaind · 2022-07-13T17:14:23Z

part of Mobile SDK overhead measurement team-mobile#5
closes Measure Sentry.init impact on app startup time #1965
resolves (partially) [Android]: Report SDK size as a comment in PRs #1303

Approach and scope of this PR

See my notes below on Macrobenchmark - after facing issues trying to get it to work on SauceLabs, I have pivoted towards Appium as it can be used to run both iOS and Android, and should also be usable for our other SDKs: Flutter, Unity, React-Native)

two simple apps (created with Android Studio -> New Project -> Basic Activity, without any more changes), the only difference being Sentry added in one of them.
build the apks for both apps (this can later be used for another check described in team-mobile#5
use Appium (through its Java SDK) on SauceLabs to launch both apps multiple times and measure the difference between them and also check sizes
add simple absolute-value tests (not comparing to values from the main branch yet)
print results in CI - the current output looks like this ("filtered" = outliers removed using the interquartile range):

[0-0] App io.sentry.java.tests.perf.appplain  launch times (original) | mean: 937.82 ms | stddev: 45.45 | values: [1038,891,901,946,918,907,911,922,945,1070,952,946,922,902,969,932,939,883,919,941,900,891,960,928,900,1015,1024,998,968,958,946,967,1010,921,965,904,1061,920,957,912,910,882,905,922,919,888,933,899,894,880]
[0-0] App io.sentry.java.tests.perf.appplain  launch times (filtered) | mean: 932.50 ms | stddev: 37.99 | values: [880,882,883,888,891,891,894,899,900,900,901,902,904,905,907,910,911,912,918,919,919,920,921,922,922,922,928,932,933,939,941,945,946,946,946,952,957,958,960,965,967,968,969,998,1010,1015,1024,1038]
[0-0] App io.sentry.java.tests.perf.appsentry launch times (original) | mean: 1004.14 ms | stddev: 24.21 | values: [1030,1017,948,1000,1034,966,1039,979,1008,995,1024,939,966,1010,1029,1018,1000,1027,993,993,975,1060,982,1030,1036,1016,1008,1023,1012,1034,987,1000,996,1012,1029,1032,1003,996,1013,984,985,1017,1001,976,992,1031,995,975,993,999]
[0-0] App io.sentry.java.tests.perf.appsentry launch times (filtered) | mean: 1005.47 ms | stddev: 22.57 | values: [948,966,966,975,975,976,979,982,984,985,987,992,993,993,993,995,995,996,996,999,1000,1000,1000,1001,1003,1008,1008,1010,1012,1012,1013,1016,1017,1017,1018,1023,1024,1027,1029,1029,1030,1030,1031,1032,1034,1034,1036,1039,1060]
[0-0] App io.sentry.java.tests.perf.appsentry takes approximately  72 ms more time to start than app io.sentry.java.tests.perf.appplain
[0-0] App io.sentry.java.tests.perf.appplain  size is 1.64MB
[0-0] App io.sentry.java.tests.perf.appsentry size is 3.56MB
[0-0] App io.sentry.java.tests.perf.appsentry is 1.92MB larger than app io.sentry.java.tests.perf.appplain

Next steps

post comment to PR - CI: performance impact measurements #2171 (comment)
test apk size difference
collect the stats somewhere (maybe even Sentry transactions) so that later we can also fail if the time grows significantly
adapt the Appium project to support iOS (minor changes) and likely move to a separate repo/action

Notes on Macrobenchmark

I have first tried using jetpack macrobenchmark by deploying the app from getsentry/sentry-android-gradle-plugin#317 to SauceLabs - the only option that seemed like it could work was using the espresso configuration for SauceLabs - it didn't though work though, the error was as follows, regardless of the config I tried (current state in https://github.com/vaind/sentry-android-gradle-plugin/commits/perf/benchmark-app-startup-time - if anyone wants to give it a try).

ERROR: Benchmark Target is Debuggable
Target package io.sentry.samples.instrumentation
is running with debuggable=true, which drastically reduces
runtime performance in order to support debugging features. Run
benchmarks with debuggable=false. Debuggable affects execution speed
in ways that mean benchmark improvements might not carry over to a
real user's experience (or even regress release performance).

While you can suppress these errors (turning them into warnings)
PLEASE NOTE THAT EACH SUPPRESSED ERROR COMPROMISES ACCURACY

// Sample suppression, in a benchmark module's build.gradle:
android {
defaultConfig {
testInstrumentationRunnerArguments["androidx.benchmark.suppressErrors"] = "DEBUGGABLE"
}
}
at androidx.benchmark.ConfigurationErrorKt.checkAndGetSuppressionState(ConfigurationError.kt:124)
at androidx.benchmark.macro.MacrobenchmarkKt.checkErrors(Macrobenchmark.kt:95)
at androidx.benchmark.macro.MacrobenchmarkKt.macrobenchmark(Macrobenchmark.kt:126)
at androidx.benchmark.macro.MacrobenchmarkKt.macrobenchmarkWithStartupMode(Macrobenchmark.kt:301)
at androidx.benchmark.macro.junit4.MacrobenchmarkRule.measureRepeated(MacrobenchmarkRule.kt:106)
at io.sentry.samples.instrumentation.benchmark.ExampleStartupBenchmark.startup(ExampleStartupBenchmark.kt:33)

#skip-changelog

codecov-commenter · 2022-07-14T12:53:02Z

Codecov Report

Base: 80.62% // Head: 80.62% // No change to project coverage 👍

Coverage data is based on head (a43974f) compared to base (de74337).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #2171   +/-   ##
=========================================
  Coverage     80.62%   80.62%           
  Complexity     3358     3358           
=========================================
  Files           240      240           
  Lines         12337    12337           
  Branches       1639     1639           
=========================================
  Hits           9947     9947           
  Misses         1783     1783           
  Partials        607      607

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

philipphofmann · 2022-07-18T09:09:22Z

I think it would be nice to use the official Android lib recommended by Google for benchmarking the app start time.
@romtsn, as you are one of our Gradle experts, could you maybe look at why the Jetpack Macrobenchmark lib used the debug config as pointed out by @vaind?
If it doesn't work out, we can use a different approach as suggested by you @vaind, of course.

vaind · 2022-07-18T17:09:40Z

I think it would be nice to use the official Android lib recommended by Google for benchmarking the app start time. @romtsn, as you are one of our Gradle experts, could you maybe look at why the Jetpack Macrobenchmark lib used the debug config as pointed out by @vaind? If it doesn't work out, we can use a different approach as suggested by you @vaind, of course.

FYI there was also some discussion on Discord: https://discord.com/channels/621778831602221064/621783562739384331/996842523446222909

vaind · 2022-07-19T11:36:34Z

Any hints how to fix the CI build failure?

FAILURE: Build failed with an exception.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
* What went wrong:
Could not determine the dependencies of task ':performance-tests:test-app-sentry:compileDebugJavaWithJavac'.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.4.2/userguide/command_line_interface.html#sec:command_line_warnings
6 actionable tasks: 6 up-to-date
> Could not resolve all task dependencies for configuration ':performance-tests:test-app-sentry:debugCompileClasspath'.
   > Could not resolve project :sentry-android.
     Required by:
         project :performance-tests:test-app-sentry
      > No matching variant of project :sentry-android was found. The consumer was configured to find an API of a component, preferably optimized for Android, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug', attribute 'com.android.build.api.attributes.AgpVersionAttr' with value '7.2.0' but:
          - Variant 'releaseApiElements' capability io.sentry:sentry-android:6.2.1 declares an API of a component, preferably optimized for Android, as well as attribute 'com.android.build.api.attributes.AgpVersionAttr' with value '7.2.0':
              - Incompatible because this component declares a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'release' and the consumer needed a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug'
          - Variant 'releaseRuntimeElements' capability io.sentry:sentry-android:6.2.1 declares a runtime of a component, preferably optimized for Android, as well as attribute 'com.android.build.api.attributes.AgpVersionAttr' with value '7.2.0':
              - Incompatible because this component declares a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'release' and the consumer needed a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug'
              ```

romtsn · 2022-07-19T21:10:22Z

Any hints how to fix the CI build failure?

FAILURE: Build failed with an exception.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
* What went wrong:
Could not determine the dependencies of task ':performance-tests:test-app-sentry:compileDebugJavaWithJavac'.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.4.2/userguide/command_line_interface.html#sec:command_line_warnings
6 actionable tasks: 6 up-to-date
> Could not resolve all task dependencies for configuration ':performance-tests:test-app-sentry:debugCompileClasspath'.
   > Could not resolve project :sentry-android.
     Required by:
         project :performance-tests:test-app-sentry
      > No matching variant of project :sentry-android was found. The consumer was configured to find an API of a component, preferably optimized for Android, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug', attribute 'com.android.build.api.attributes.AgpVersionAttr' with value '7.2.0' but:
          - Variant 'releaseApiElements' capability io.sentry:sentry-android:6.2.1 declares an API of a component, preferably optimized for Android, as well as attribute 'com.android.build.api.attributes.AgpVersionAttr' with value '7.2.0':
              - Incompatible because this component declares a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'release' and the consumer needed a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug'
          - Variant 'releaseRuntimeElements' capability io.sentry:sentry-android:6.2.1 declares a runtime of a component, preferably optimized for Android, as well as attribute 'com.android.build.api.attributes.AgpVersionAttr' with value '7.2.0':
              - Incompatible because this component declares a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'release' and the consumer needed a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug'
              ```

It's likely because you also need to exclude them for bom and distribution, like we did with sentry-test-support:

sentry-java/sentry-bom/build.gradle.kts

Lines 6 to 18 in 121d465

    
           dependencies { 
        
               constraints { 
        
                   project.rootProject.subprojects 
        
                       .filter { 
        
                           !it.name.startsWith("sentry-samples") && 
        
                               it.name != project.name && 
        
                               it.name != "sentry-test-support" 
        
                       } 
        
                       .forEach { 
        
                           api(it) 
        
                       } 
        
               } 
        
           }

and

sentry-java/build.gradle.kts

Line 101 in 121d465

    
           if (!this.name.contains("sample") && !this.name.contains("integration-tests") && this.name != "sentry-test-support") {

romtsn · 2022-07-19T21:11:26Z

@vaind sorry man, I haven't got time to look at the PR, so I will leave this to @marandaneto as I'm on parental leave starting from tomorrow. Good luck!

vaind · 2022-07-20T07:38:10Z

@vaind sorry man, I haven't got time to look at the PR, so I will leave this to @marandaneto as I'm on parental leave starting from tomorrow. Good luck!

thanks and enjoy the time with your kid/s

Because the latter doesn't work on saucelabs with `storage:` specifier. See https://support.saucelabs.com/hc/en-us/requests/123037

See the overall issue at getsentry/team-mobile#5 and the sibling PR at sentry-java for the approach & some more details: getsentry/sentry-java#2171

github-actions · 2022-09-08T21:43:31Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time (ms)	370.61	399.90	29.29
Size (bytes)	1825823	2445266	619443

sentry-android-integration-tests/test-app-plain/build.gradle.kts

marandaneto · 2022-09-09T08:34:19Z

@vaind I left a comment, is there anything pending or ready to merge?

marandaneto · 2022-09-09T09:39:00Z

sentry-android-integration-tests/test-app-plain/build.gradle.kts

+    }
+}
+
+dependencies {


The Sentry Android Core lib has the dependencies of:

// lifecycle processor, session tracking implementation(Config.Libs.lifecycleProcess) implementation(Config.Libs.lifecycleCommonJava8) implementation(Config.Libs.androidxCore)

But most if not all Apps also have them as well, what if we add those dependencies in the plain test so they are not counted for the diff in size?

Hmm, personally I'm not sure about that - you'd know better. Maybe we can try in a followup PR what's the difference?

sounds good, so we unblock this PR.

sentry-android-integration-tests/test-app-sentry/build.gradle.kts

vaind · 2022-09-17T08:20:09Z

@vaind I left a comment, is there anything pending or ready to merge?

Good to go from my POV

marandaneto

Thank you @vaind

vaind force-pushed the ci/perf-tests branch from da80e96 to 800c5cc Compare July 14, 2022 16:03

vaind force-pushed the ci/perf-tests branch from ce3cbeb to 104f178 Compare July 18, 2022 11:33

vaind force-pushed the ci/perf-tests branch from ea6260f to b033f2a Compare July 19, 2022 11:36

vaind and others added 20 commits July 20, 2022 09:38

test: appium-based android startup test (WIP)

466d695

chore: update appium test dependencies

be814f5

test: add performance test apps

5de4fcc

chore: perf test apps - add signing configs

3fe51dc

test: use appId

cca02ea

Format code

3693a71

refactor: switch appium tests to TS

93eaf86

chore: appium TS package cleanup

dcc86bf

chore: finish migration to nodejs appium

08839af

chore: use simple-statistics in the appium test

7413898

chore: lints

d7cb38a

chore: update appium test to multi-app

1f2973f

chore: update appium test scripts

d1f36f7

wip: upload apps to saucelabs

31259c1

make gradlew executable in test apps

99650f9

fixup: saucelabs app upload

b2f337f

refactor: use appium:otherApps instead of installApp

d935121

Because the latter doesn't work on saucelabs with `storage:` specifier. See https://support.saucelabs.com/hc/en-us/requests/123037

ci: run appium perf test in CI

4954245

test: add sentry to the startup test app

f4850c6

Format code

891f3ee

vaind added 2 commits August 12, 2022 10:27

chore: update metrics GHA

5e9c63a

update perf test config

935e208

This was referenced Aug 12, 2022

Perf metrics getsentry/sentry-cocoa#2067

Merged

Ci/perf tests getsentry/sentry-react-native#2420

Closed

vaind and others added 3 commits August 14, 2022 12:33

ci: cache benchmark plain app

9e63c21

switch metrics action to internal repo

f6723f6

Merge branch 'main' into ci/perf-tests

3f8cf60

vaind marked this pull request as ready for review August 17, 2022 14:57

vaind requested review from adinauer and romtsn as code owners August 17, 2022 14:57

vaind mentioned this pull request Aug 21, 2022

Ci/perf tests getsentry/sentry-react-native#2430

Merged

8 tasks

philipphofmann pushed a commit to getsentry/sentry-cocoa that referenced this pull request Aug 31, 2022

Perf metrics (#2067)

be9ff08

See the overall issue at getsentry/team-mobile#5 and the sibling PR at sentry-java for the approach & some more details: getsentry/sentry-java#2171

Merge branch 'main' into ci/perf-tests

a43974f

vaind force-pushed the ci/perf-tests branch from 05d4021 to a43974f Compare September 7, 2022 15:29

getsentry deleted a comment from github-actions bot Sep 9, 2022

marandaneto reviewed Sep 9, 2022

View reviewed changes

sentry-android-integration-tests/test-app-plain/build.gradle.kts Outdated Show resolved Hide resolved

Merge branch 'main' into ci/perf-tests

ffeb2a5

marandaneto reviewed Sep 9, 2022

View reviewed changes

sentry-android-integration-tests/test-app-sentry/build.gradle.kts Show resolved Hide resolved

vaind added 5 commits September 16, 2022 14:41

chore: build only arm64 metrics test apps

0e00c1a

chore: use Config.Android version constants in the metrics test apps

da971d2

fix: metrics test apps compat with old android

059f3ea

test: metrics binary size test range

f40bb1a

fix: metrics test apps old android compat

fde4c27

marandaneto approved these changes Sep 17, 2022

View reviewed changes

marandaneto merged commit 429f878 into main Sep 17, 2022

marandaneto deleted the ci/perf-tests branch September 17, 2022 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: performance impact measurements #2171

CI: performance impact measurements #2171

vaind commented Jul 13, 2022 •

edited

Loading

codecov-commenter commented Jul 14, 2022 •

edited

Loading

philipphofmann commented Jul 18, 2022

vaind commented Jul 18, 2022

vaind commented Jul 19, 2022 •

edited

Loading

romtsn commented Jul 19, 2022

romtsn commented Jul 19, 2022

vaind commented Jul 20, 2022

github-actions bot commented Sep 8, 2022 •

edited

Loading

marandaneto commented Sep 9, 2022

marandaneto Sep 9, 2022

vaind Sep 16, 2022 •

edited

Loading

marandaneto Sep 17, 2022

vaind commented Sep 17, 2022

marandaneto left a comment

CI: performance impact measurements #2171

CI: performance impact measurements #2171

Conversation

vaind commented Jul 13, 2022 • edited Loading

Approach and scope of this PR

Next steps

Notes on Macrobenchmark

codecov-commenter commented Jul 14, 2022 • edited Loading

Codecov Report

philipphofmann commented Jul 18, 2022

vaind commented Jul 18, 2022

vaind commented Jul 19, 2022 • edited Loading

romtsn commented Jul 19, 2022

romtsn commented Jul 19, 2022

vaind commented Jul 20, 2022

github-actions bot commented Sep 8, 2022 • edited Loading

Performance metrics 🚀

marandaneto commented Sep 9, 2022

marandaneto Sep 9, 2022

Choose a reason for hiding this comment

vaind Sep 16, 2022 • edited Loading

Choose a reason for hiding this comment

marandaneto Sep 17, 2022

Choose a reason for hiding this comment

vaind commented Sep 17, 2022

marandaneto left a comment

Choose a reason for hiding this comment

vaind commented Jul 13, 2022 •

edited

Loading

codecov-commenter commented Jul 14, 2022 •

edited

Loading

vaind commented Jul 19, 2022 •

edited

Loading

github-actions bot commented Sep 8, 2022 •

edited

Loading

vaind Sep 16, 2022 •

edited

Loading