epicadk commented 2 years ago

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #825

Description Clear and concise code change description.

Setup benchmark module for benchmarking fhir engine operations.

Alternative(s) considered Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

nil

Type Choose one: Feature Screenshots (if applicable)

Checklist

[x] I have read and acknowledged the Code of conduct
[x] I have read How to Contribute
[x] I have read the Developer's guide
[x] I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA
[ ] I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach
[x] I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project
[x] I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally
[x] I have built and run the reference app(s) to verify my change fixes the issue and/or does not break the reference app(s)

epicadk commented 2 years ago

That's odd the tests seem to run fine on when I run it from Android studio however ./gradlew benchmark:ConnectedCheck throws an error.

epicadk commented 2 years ago

Still trying to figure out why the default output directory is not writable.

epicadk commented 2 years ago

The benchmark should run and it will still display the overall result in the log. However there seems to be an issue with the read and write permissions, so currently we're unable to generate the detailed benchmark report. I've followed this user guide, however it still throws an error while trying to generate the report. cc @jingtang10 @yigit

codecov-commenter commented 2 years ago

Codecov Report

Merging #822 (3b8334d) into master (6e336b3) will decrease coverage by 51.70%. The diff coverage is n/a.

:exclamation: Current head 3b8334d differs from pull request most recent head 9bb2fbb. Consider uploading reports for the commit 9bb2fbb to get more accurate results

@@              Coverage Diff              @@
##             master     #822       +/-   ##
=============================================
- Coverage     84.57%   32.86%   -51.71%     
+ Complexity      651      460      -191     
=============================================
  Files           140      109       -31     
  Lines         10498     9030     -1468     
  Branches        796      561      -235     
=============================================
- Hits           8879     2968     -5911     
- Misses         1220     5758     +4538     
+ Partials        399      304       -95

Impacted Files	Coverage Δ
...a/com/google/android/fhir/sync/FhirSynchronizer.kt	`67.03% <0.00%> (-14.55%)`	:arrow_down:
...ws/QuestionnaireItemRadioGroupViewHolderFactory.kt	`62.00% <0.00%> (-11.34%)`	:arrow_down:
...e/views/QuestionnaireItemGroupViewHolderFactory.kt	`50.00% <0.00%> (-10.87%)`	:arrow_down:
...ava/com/google/android/fhir/search/NestedSearch.kt	`90.00% <0.00%> (-10.00%)`	:arrow_down:
...iews/QuestionnaireItemDropDownViewHolderFactory.kt	`41.17% <0.00%> (-8.83%)`	:arrow_down:
...droid/fhir/datacapture/QuestionnaireItemAdapter.kt	`52.30% <0.00%> (-8.81%)`	:arrow_down:
...re/validation/ValueConstraintExtensionValidator.kt	`91.66% <0.00%> (-8.34%)`	:arrow_down:
...om/google/android/fhir/db/impl/DbTypeConverters.kt	`31.25% <0.00%> (-7.64%)`	:arrow_down:
...oid/fhir/search/filter/DateParamFilterCriterion.kt	`80.76% <0.00%> (-7.33%)`	:arrow_down:
...java/com/google/android/fhir/resource/Resources.kt	`53.84% <0.00%> (-6.16%)`	:arrow_down:
... and 101 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e22e20a...9bb2fbb. Read the comment docs.

epicadk commented 2 years ago

Not sure why the test is failing on android 10

Tarun-Bhardwaj commented 2 years ago

Not sure why the test is failing on android 10

@epicadk , did you get a chance to resolve this issue?

epicadk commented 2 years ago

Not sure why the test is failing on android 10

@epicadk , did you get a chance to resolve this issue?

I tried following the docs and codelabs but can't seem to fix this. Will give it another try this week.

epicadk commented 2 years ago

@yigit could you please take a look at this?

cc @jingtang10

epicadk commented 2 years ago

The error is

Failed to pull \storage\emulated\0\Android\data\com.google.android.fhir.benchmark.test\files\test_data\FhirEngineBenchMark__index_patient-stackSamplingLegacy-2022-02-25-04-51-08.trace from device

However the file exists on the device

Tarun-Bhardwaj commented 2 years ago

@epicadk , do you have an ETA to complete the changes for this PR? This is blocking issue #1100

epicadk commented 2 years ago

The benchmarks run in the IDE with output disabled. Still trying to figure out why they won't run when we enable the output.

epicadk commented 2 years ago

https://issuetracker.google.com/issues/194525079#comment2 might be relevant.

epicadk commented 2 years ago

quite flakey the tests passed before this but the upload failed. When I ran the tests again they failed with a timeout exception. Although I think this may have something to do with the fact that we are running it on an emulator.

vitorpamplona commented 2 years ago

Hi all,

What's the status of this PR? Are we moving forward with this Benchmark module idea?

jingtang10 commented 2 years ago

closing in favour of #1414

google / android-fhir

setup benchmark module #822

Codecov Report