-
Notifications
You must be signed in to change notification settings - Fork 15
Enable R8 optimizations. #476
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
Conversation
a2fd5a5 to
5cafdb7
Compare
JaCoCo Code Coverage 93.98% ✅
Generated by 🚫 Danger |
📊 Benchmark Comparison ReportSummary
No significant changes (25 benchmarks)
Regression threshold: ±10% with non-overlapping confidence intervals Generated by 🚫 Danger |
5cafdb7 to
5e79884
Compare
| @@ -0,0 +1,72 @@ | |||
| name: R8 Integration Tests | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running emulator tests on github CI
24062b9 to
fc5e784
Compare
|
|
||
| object AbstractClassInheritanceFormulas { | ||
|
|
||
| class One : ActionFormula<Unit, Int>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both One and Two extend ActionFormula. In our test, we want to make sure that IFormula.type() implementation is not modified by R8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the primary issue that I noticed with fullMode=true enabled
bf0dcf7 to
1587859
Compare
| @@ -0,0 +1,70 @@ | |||
| plugins { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configuring tests with R8 enabled
| @@ -0,0 +1,9 @@ | |||
| # Formula R8/ProGuard Rules | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be packaged with the jar. Without this, the test fails with
com.instacart.formula.r8.FormulaR8Test > actionFormulas[Medium_Phone_API_36.1(AVD) - 16] FAILED
R.a: Formula class S.a with a key FormulaKey(scopeKey=null, type=class S.a, key=kotlin.Unit) already requested by class com.instacart.formula.r8.interactors.AbstractFormulaTypeInheritanceInteractor$formula$1
at Q.e.a(SourceFile:74)
Tests on Medium_Phone_API_36.1(AVD) - 16 failed: There was 1 failure(s).
1587859 to
a4d9eaf
Compare
jasonostrander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What
Adding official support for R8 optimizations for formula