Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Document how release builds differ from local debug builds #6931

Closed
mcomella opened this issue Dec 3, 2019 · 8 comments
Closed

Document how release builds differ from local debug builds #6931

mcomella opened this issue Dec 3, 2019 · 8 comments
Assignees
Labels
performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Dec 3, 2019

What

Ideally, debug builds are as close as possible to release builds so that the code developers run daily matches what users see. Currently, that is not the case. For example, telemetry is disabled in debug builds #6447.

While we can make the builds the same, the lightweight step is to document all the differences so at least developers are aware of them.

Impact

For performance, the builds we usually profile will not represent what users see so we may miss critical performance issues or make changes to a build that do not affect the user experience. If these are documented, we're less likely to make these mistakes.

Acceptance criteria

  • Each difference between debug builds (which variant?) and the release builds is documented and shared with the performance team
  • Make necessary changes based on the distinction (if it's not too time intensive) or file a follow-up for reprioritization

┆Issue is synchronized with this Jira Task

@mcomella mcomella added the performance Possible performance wins label Dec 3, 2019
@mcomella mcomella added this to Needs prioritization in Performance, front-end roadmap via automation Dec 3, 2019
@hawkinsw
Copy link
Contributor

hawkinsw commented Dec 5, 2019

  1. GeckoView will load a configuration file at runtime at the time of its initialization (blocking) if the application is 'debuggable'. https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java#322
  2. In setDayNightTheme when running a Performance Test build, the Sentry is invoked to capture a "crash" (that variant is not considered valid and, therefore, Fenix generates a crash report). This causes significant slowdown.

@mcomella mcomella moved this from Needs prioritization to Backlog (prioritized) in Performance, front-end roadmap Dec 9, 2019
@mcomella
Copy link
Contributor Author

In Fire TV, so maybe here, Debug builds did not automatically opt into experiments but release builds did: I wonder how different the code is that runs.

@mcomella mcomella self-assigned this Dec 16, 2019
@hawkinsw
Copy link
Contributor

  1. Fenix will print a stack trace every time that Fenix violates Strict Mode. This is useful for debugging, but results in performance variability. There can be differences in performance if/when the stacks differ between versions where there are Strict Mode violations (ie, deeper stacks will result in longer times to print the stack trace)

@mcomella
Copy link
Contributor Author

mcomella commented Dec 20, 2019

The documentation has been completed. Remaining action items:

  • Notify team on build variant recommendations & update documentation
  • File issues for follow-up action items (these will be linked from the document)

Brief summary

Goal: Local devs should use while analyzing and improving performance

  • Requirements: debuggable for profiling, representative of user experience
  • Recommendation: forPerformanceTest with Leanplum, Adjust, & Sentry tokens
  • Rationale: there are numerable differences in debug and release that may negatively affect performance so release builds are the only builds that will be representative. However, we need debuggable: that leaves one choice. We need API keys to ensure the experience is fully representative

Goal: Performance automation, manual benchmarks should use

  • Requirements: ?
  • Recommendation: production builds if possible. If debuggable is necessary, use recommendation above. If forPerformanceTest is used solely for BrowserPerformanceTestActivity and not debuggable, we should find a new solution without debuggable
  • Rationale: like the above, we need release builds so the choice is based on the debuggable flag.

@mcomella
Copy link
Contributor Author

I created a PR to add the recommendation to the README: #7315

@mcomella
Copy link
Contributor Author

I finished filing issues for action items and linked them from the analysis.

@mcomella
Copy link
Contributor Author

Once the PR lands, the remaining action item is to email the relevant parties.

@mcomella
Copy link
Contributor Author

mcomella commented Dec 26, 2019

I'll handle the email in #6464 (the follow-up issue): closing as FIXED.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance Possible performance wins
Development

No branches or pull requests

2 participants