Skip to content
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

ci: benchmarking and profile generation improvements #2022

Merged
merged 30 commits into from
Jul 29, 2022

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Jul 28, 2022

  • The profile generation workflow is flaking a lot. This simplifies the test logic and makes more granular XCUIElementQueries so we can better pinpoint failures.
    • There seem to be two failure modes happening:
      1. Can't find back button when it thinks it's in a movie detail view but it's actually already back on the movie list.
        • ✅ In this case, when it doesn't find a back button, we check if the tab bar is visible to verify if we're already back on the list.
      2. Can't find the back button when it goes to a movie detail and the app crashes. Seems to always happen right after navigating to the last tab bar button item.
        • ❌ tried converting all fatalErrors to print statements
        • ❌ tried disabling automatic UIControl tracing, since we define our own spans
        • ❌ try fixing some possible nil dereferences uncovered by thread sanitizer
        • ❌ retry everything 5 times with saucectl
        • ➡️ we just won't generate profiles from low-end devices, it's the only one that consistently fails
  • Fixes a bug where the TrendingMovies carthage dependencies are rebuilt even with cache hits. Takes job time from 13 down to 4.5 minutes.
  • Don't upload dSYMs for profile generation test runner app; we only need one for the app under test it's driving.
  • Standardize on using one device from each class we have (recently updated in fix: update ios device classification sentry#37044)
    • Try twiddling some things to get the low-end device to pass–it consistently fails to start the test process.
  • Assert benchmark overhead result is > 0%; this started happening due to some bug and the test happily passed since it's < 5%.
  • After changing the benchmark to start directly when the performance VC is presented, in combination with the new option to automatically track VC performance, the profiler would stop soon after beginning due to the idle timeout. disabled auto tracing to prevent this behavior (this happened before with the button press to start the benchmark in combination with enableUserInteractionTracing)
  • Fixed another edge case in the benchmark report generation script for various ways xcuitest log lines are exported with newlines and escaped newlines

#skip-changelog

@armcknight armcknight force-pushed the armcknight/fix-profile-generation branch from b8fa8a5 to 146cd7e Compare July 28, 2022 18:44
@armcknight armcknight changed the title ci: simplify profile generation UI test logic ci: benchmarking and profile generation improvements Jul 28, 2022
@armcknight armcknight force-pushed the armcknight/fix-profile-generation branch from ca0a06f to 62f77d1 Compare July 28, 2022 23:51
@armcknight armcknight force-pushed the armcknight/fix-profile-generation branch from 3e6df63 to 49200dc Compare July 29, 2022 04:45
@armcknight armcknight force-pushed the armcknight/fix-profile-generation branch from 9a823a9 to 1792c5d Compare July 29, 2022 05:17
@armcknight armcknight marked this pull request as ready for review July 29, 2022 16:44
@armcknight armcknight force-pushed the armcknight/fix-profile-generation branch from 83e9ef9 to a6e138b Compare July 29, 2022 16:51
@@ -209,7 +212,9 @@ - (void)start
- (void)stop
{
@synchronized(self) {
_profiler->stopSampling();
if (_profiler != nullptr) {
Copy link
Member

Choose a reason for hiding this comment

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

I remember hitting a crash here earlier, thanks for fixing this

@armcknight armcknight merged commit 29fe666 into master Jul 29, 2022
@armcknight armcknight deleted the armcknight/fix-profile-generation branch July 29, 2022 18:13
kevinrenskers added a commit that referenced this pull request Aug 1, 2022
* master:
  ci: benchmarking and profile generation improvements (#2022)
  release: 7.23.0
  fix: Bad merge on CHANGELOG.md (#2020)
  fix: Log empty samples instead of collecting stacks for idle threads (#2013)
  fix: Handle failure to read thread priority gracefully (#2015)
  fix: Remove logging that could occur while a thread is suspended (#2014)
  fix: Use constant for max thread name size instead of magic number (#2012)
  Disable flaky tesDefaultMaxEnvelopesConcurrent (#2018)
  Fix build failure in unit tests (#2019)
  Fix address sanitizer compilation error (#1996)
  fix: Sauce labs iPhone 13 pro iOS version
  feat: Add transaction to baggage and trace headers (#1992)
  ci: fix ui thread starvation in benchmarks (#2009)
  fix: Add more descriptive deprecation message for enableProfiling (#2011)
  feat: Add sampling configuration for profiling (#2004)
  fix log message option name (#2006)
  meta: add armcknight as a codeowner (#2008)
  release: 7.22.0
  fix: Disable broken LaunchUITests to unblock release (#2003)

# Conflicts:
#	Samples/iOS-Swift/iOS-Swift/ViewControllers/PerformanceViewController.swift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants