[DO NOT MERGE] Temp check build stats with math prs #4261
Conversation
…nd-eval' into add-support-for-math-expressions-pt9-commutative-comparisons Conflicts: utility/src/main/java/org/oppia/android/util/math/RealExtensions.kt utility/src/test/java/org/oppia/android/util/math/BUILD.bazel
This inlines three recursive operations to be done during the actual computation to simplify the overall converter complexity (and to make determining the test matrix easier).
…isons-protos' into add-support-for-math-expressions-pt5-polynomial-protos
… into add-support-for-math-expressions-pt6-tokenizer Conflicts: scripts/assets/test_file_exemptions.textproto
…d-support-for-math-expressions-pt7-math-expression-parser
…rser' into add-support-for-math-expressions-pt8-latex-conversion-and-eval
…nd-eval' into add-support-for-math-expressions-pt9-commutative-comparisons
This adjusts for the removal of ComparableOperationList (i.e. no wrapper proto).
This includes fixes to the converter itself as it wasn't distributing both product inversions and negation correctly in several cases. Tests should now be covering these cases.
Still needs to be cleaned up, but after converter refactoring attempts.
…isons' into add-support-for-math-expressions-pt10-polynomials Conflicts: utility/src/main/java/org/oppia/android/util/math/MathExpressionExtensions.kt utility/src/main/java/org/oppia/android/util/math/PolynomialExtensions.kt utility/src/main/java/org/oppia/android/util/math/RealExtensions.kt
Also, clean up polynomial sorting.
… add-support-for-math-expressions-pt11-numeric-expression-input-classifiers Conflicts: utility/src/main/java/org/oppia/android/util/math/MathExpressionExtensions.kt utility/src/main/java/org/oppia/android/util/math/PolynomialExtensions.kt utility/src/main/java/org/oppia/android/util/math/RealExtensions.kt
Also, mark methods/classes that need tests.
… into add-support-for-math-expressions-pt6-tokenizer
…into refactor-async-result
…aic-expression-system
Conflicts: WORKSPACE third_party/maven_install.json
…build-stats-with-math-prs
…s-with-math-prs Conflicts: WORKSPACE config/proguard/kotlin-proguard-rules.pro third_party/maven_install.json
This PR resolves *all* Maven conflicts and Proguard issues by ensuring everything is using the latest Kotlin runtime & libraries. Tests have not been verified as building yet (they probably won't due to visibility changes & new behaviors/API changes introduced in Kotlin 1.5 and 1.6). This is needed as part of checking #2173 for build stats (which will happen on another, temporary branch).
…s-with-math-prs Includes a regeneration of maven_install.json. Conflicts: app/build.gradle config/proguard/kotlin-proguard-rules.pro third_party/BUILD.bazel third_party/maven_install.json third_party/versions.bzl utility/build.gradle utility/src/main/java/org/oppia/android/util/parser/image/UrlImageParser.kt
BenHenning
requested review from
anandwana001,
vinitamurthi and
rt4914
as code owners
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 13 MiB (old), 12 MiB (new), 612 KiB (Removed) APK download size (estimated): 12 MiB (old), 11 MiB (new), 689 KiB (Removed) Method count: 167558 (old), 142651 (new), 24907 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5034 (old), 5133 (new), 99 (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added):
AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 13 MiB (old), 12 MiB (new), 612 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration hdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 54 KiB (old), 54 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 51 KiB (old), 51 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 100 KiB (old), 100 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 65 KiB (old), 65 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 74 KiB (old), 74 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 77 KiB (old), 77 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 7932 KiB (old), 7926 KiB (new), 6963 bytes (Removed) APK download size (estimated): 6845 KiB (old), 6768 KiB (new), 77 KiB (Removed) Method count: 70879 (old), 62087 (new), 8792 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 4177 (old), 4276 (new), 99 (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added):
AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 7700 KiB (old), 7694 KiB (new), 6963 bytes (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration hdpiAPK file size: 51 KiB (old), 51 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 88 KiB (old), 88 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 59 KiB (old), 59 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 70 KiB (old), 70 KiB (new), 0 bytes (No change) Alpha_kitkatExpand to see flavor specificsUniversal APKAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed) APK download size (estimated): 12 MiB (old), 11 MiB (new), 712 KiB (Removed) Method count: 168318 (old), 142805 (new), 25513 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5034 (old), 5133 (new), 99 (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added):
AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration hdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 54 KiB (old), 54 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 51 KiB (old), 51 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 100 KiB (old), 100 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 65 KiB (old), 65 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 74 KiB (old), 74 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 77 KiB (old), 77 KiB (new), 0 bytes (No change) Configuration standalone-hdpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-ldpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-mdpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-tvdpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-xhdpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-xxhdpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-xxxhdpiAPK file size: 13 MiB (old), 12 MiB (new), 637 KiB (Removed)
Lesson assets: 71 (old), 73 (new), 2 (Added) |
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 12 MiB (new), 972 KiB (Added) APK download size (estimated): 10 MiB (old), 11 MiB (new), 925 KiB (Added) Method count: 135895 (old), 142651 (new), 6756 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5034 (old), 5133 (new), 99 (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added):
AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 12 MiB (new), 972 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration hdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 54 KiB (old), 54 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 51 KiB (old), 51 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 100 KiB (old), 100 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 65 KiB (old), 65 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 74 KiB (old), 74 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 77 KiB (old), 77 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 7186 KiB (old), 7926 KiB (new), 739 KiB (Added) APK download size (estimated): 6069 KiB (old), 6768 KiB (new), 698 KiB (Added) Method count: 58280 (old), 62087 (new), 3807 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 4177 (old), 4276 (new), 99 (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added):
AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 6954 KiB (old), 7694 KiB (new), 739 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration hdpiAPK file size: 51 KiB (old), 51 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 88 KiB (old), 88 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 59 KiB (old), 59 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 68 KiB (old), 68 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 70 KiB (old), 70 KiB (new), 0 bytes (No change) Alpha_kitkatExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added) APK download size (estimated): 10 MiB (old), 11 MiB (new), 919 KiB (Added) Method count: 136310 (old), 142805 (new), 6495 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5034 (old), 5133 (new), 99 (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added):
AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration hdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 54 KiB (old), 54 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 51 KiB (old), 51 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 100 KiB (old), 100 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 65 KiB (old), 65 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 74 KiB (old), 74 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 77 KiB (old), 77 KiB (new), 0 bytes (No change) Configuration standalone-hdpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-ldpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-mdpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-tvdpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-xhdpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-xxhdpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) Configuration standalone-xxxhdpiAPK file size: 11 MiB (old), 12 MiB (new), 967 KiB (Added)
Lesson assets: 71 (old), 73 (new), 2 (Added) |
|
Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
oppiabot
bot
added
the
stale
|
This PR has served its purpose, and the math PRs have been merged. Closing this. |
## Explanation Fixes part of #1719 and #3709 This PR introduces a new script & CI workflow for computing build stats to compare both AABs and universal APKs between develop and the changes in a given PR, as part of fixing #1719 (though this PR doesn't cover everything outlined in that PR). This information is then detailed and uploaded as a CI build artifact, and summarized & posted as a comment in the PR. Some details included in the summary report: - APK file/download size differences - Method count differences - Feature/permission differences - New/removed resources & assets The script supports computing differences for multiple "profiles" at the same time, and the CI workflow has been set up to compute four: 1. dev 2. alpha 3. beta 4. GA This workflow will be optional since it's very expensive to run (it has to assemble 8 builds, 6 of which are Proguarded). It also doesn't really need to be run in order to approve a PR, though reviewers may insist on waiting for large or suspicious changes (such as PRs introducing new dependencies) to ensure the actual affected changes are as expected. In order to mitigate this expense, the CI workflow runs on a scheduled cron job off of develop across all open PRs and checks them in a group. It runs at most once per day (based on https://github.com/orgs/community/discussions/55768#discussioncomment-5941720) so multiple changes to a PR will be picked up with a single check in the next cron run. Currently, it will run even for a PR that hasn't changed since the last run (but this is something that can be improved in the future if it needs to be). It's being scheduled for 2:30am (02:30) UTC which seems to have a few specific benefits: - Per GitHub documentation, initiating the workflow outside the start of the hour should reduce likelihood of cancellation (since the start of the hour tends to use the most resources): https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule. - This corresponds to 7:30pm PT, 2:30am GMT, 8:00am IST, 5:30am EAT, and 12:30pm AEST (just as a basis for a different part of the world). It's actually a very nice time that shouldn't overlap with almost any development in main locations around the world, so it hopefully won't impact Oppia organization GitHub CI resources. The example output from these workflows can be observed in a few places: - Later in this PR (for back when the PR was configured to run the new workflow per PR change). - In #4261 which demonstrates the large math PRs and how they changed the builds back before those were merged. - https://github.com/BenHenning/oppia-android/actions/workflows/stats.yml and https://github.com/BenHenning/oppia-android/pulls (specifically: BenHenning#14, BenHenning#13, BenHenning#12) which demonstrates the workflow running correctly from a scheduled cron (https://github.com/BenHenning/oppia-android/actions/runs/9232187176) and posting the updates to open PRs. Beyond that, implementing this utility involved several significant changes to various systems, including the build graph: - Three new utilities were added for the script: Aapt2Client, ApkAnalyzerClient, and BundleToolClient. Each of these correspond to Android CLI utilities, but each required special considerations: - Aapt2Client requires direct access to the Android SDK, but fortunately android_sdk_repository exposes this as a target so it's trivial to pass it in & call it. Some build information is needed, too (see next outer point). - ApkAnalyzerClient couldn't use the apkanalyzer CLI contained within the SDK since it's not exported by android_sdk_repository. Instead, we needed to depend on the CLI's internal implementation library (which I suspect is what Android Studio probably uses for its own APK Analyzer tool). This required some new implementation. - BundleToolClient fortunately can call right into the bundle tool library that we use when building AABs, but unfortunately that tool appears to not be designed to be called multiple times in the same process. Because Java doesn't support forking, we actually needed to fake a fork function by starting a new Java process using the current process's classpath in order to re-run bundle tool for each needed routine. Additionally, bundle tool required https://github.com/oppia/archive-patcher (which needed new BUILD files since it only supported Gradle building previously) and a non-Android version of Guava (see below for the changes this has caused). - A new build_vars.bzl was introduced to define the build SDK & build tools versions (this is done in a way where they can actually be passed to the new script's utilities since it needs to access aapt2). - rules_kotlin had a bug where resources wouldn't be pulled in properly for kt_jvm_library (see bazelbuild/rules_kotlin#281), but this was mitigated in a previous PR by upgrading rules_kotlin past alpha 2. - The new functionality required the JRE-compatible version of Guava (over the Android-constrained library used in the codebase today), but this introduces a one-version issue. The solution ended up being isolating the JRE-compatible Guava library to its own library with a slightly hacky direct reference to it in BundleToolClient. Some of the other attempts at solving this resulted in some Maven reference cleanups in existing script documentation. This functionality will be improved in downstream PRs, but other attempts that were originally made to isolate this cleanly were: - Introduce multiple maven_install files and isolate dependencies into: production, tests, scripts. This has a number of nice benefits (more correct licenses and faster Maven dependency fetches for production), but it results in very tricky one-version violations for test targets that cross dependencies between production and tests. - Isolated maven_install just for scripts. This is closer to the solution we'll want long-term, but it was too much complexity to fully introduce in this PR so it's been reworked into a downstream PR that can focus on cleaning up third-party dependency management across the whole codebase. This PR is introducing a few new dependencies that, in turn, pull in a *bunch* of transitive dependencies. These are all due to the new ``apkanalyzer`` dependency. While it will affect licenses for this specific PR, once third-party dependencies for scripts are cleaned up in a downstream PR they will be moved out (since they are script-only dependencies). Separately, also note that the AAPT2 utility requires stdout to be processed continuously in order for the process to finish. This was one of the primary reasons CommandExecutorImpl was reworked in #4929. For testing: most of the changes in this PR have been extensively manually tested. However, the new utilities are lacking significant automated tests. Since this utility is a nice-to-have for the rest of the Bazel PR chain, it's being prioritized to be merged in spite of lacking code coverage. #4971 has been filed to track adding these missing tests in the long-term. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only N/A -- This only affects CI workflows & the build system. Technically, some dependency changes in the build system could have UI effects, but there should be no such changes in this PR. --------- Co-authored-by: Adhiambo Peres <[email protected]> Co-authored-by: Sean Lip <[email protected]>
Explanation
This is a temporary integration branch of #2173 and #4092 since the latter won't be able to make it into develop before the former, and it'd be really good to see the diff stats for such a large project to see what impact it'll have.
This should be compared against #4092's own diff stats to account for the fact that it's making a lot of changes to the build graph, too (particularly around dependencies since it's upgrading the app to Kotlin 1.6).
This PR will not be merged and will be closed once the stats are computed.
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: