Move decorating events with mutable device info to a background thread
Description
This PR is related to improving performance of TracksClient#track method. This is crucial for performance of projects using this library - this method is called in numerous places, very often on the main thread. Internally we discussed and measure impact: paqN3M-18i-p2
Fix
I moved execution of the longest method, DeviceInformation#getMutableDeviceInfo, to a thread that is responsible for moving events to local database. As a result, getMutableDeviceInfo is executed a few milliseconds later than previously, but in reality it shouldn't affect business value that we gain from these properties.
Test
Performance testing
I've introduced benchmark module to perform these tests. I don't add CI check of any kind, as it's not trivial when it comes to performance benchmarking and requires rather a solid setup which holds results from many previous executions. (https://medium.com/androiddevelopers/fighting-regressions-with-benchmarks-in-ci-6ea9a14b5c71).
- Run
basicTrackMethod()on a real device - Write down the median value (see test result right panel > "Benchmark" tab)
- Revert the fix:
git revert 82f903cf53b3cfcc8f8af5d81e7e1842029943af - Run
basicTrackMethod()on a real device again - Compare results from both executions: first result should be significantly lower.
Validation testing
In this test, we want only to validate if "mutable device info" are still updated.
- I used WooCommerce for this. If https://github.com/woocommerce/woocommerce-android/pull/11416 is not yet merged, please use it as a base as it addresses breaking changes introduced with Tracks 5.0.0 release.
- Replace a version of Tracks with the one from this PR (recent:
214-5c57d4048541a07f1e7b8aa011366e6844e71aee) - Use some network intercepting software like Charles or Proxyman
- Perform some navigation (e.g. open order details): see that after some time there's a call to
https://public-api.wordpress.com/rest/v1.1/tracks/recordwithcommonPropsarray anddevice_info_device_orientationin it. Verify the value was correct. - Change phone orientation
- Perform again some operations and verify if after a few seconds there's another call to tracks, this time with opposite value for
device_info_device_orientation.
Screenshots
Those are from my performance tests. They show median execution time over multiple executions.
Before
After
Because we discussed around this topic recently, and it's relatively easy to test this change on WooCommerce - @hichamboushaba do you mind reviewing this PR?
Because we discussed around this topic recently, and it's relatively easy to test this change on WooCommerce - @hichamboushaba do you mind reviewing this PR?
Sorry for the late reply @wzieba, would it be OK to review this tomorrow or next Monday? I have to focus on finishing up a project before the code freeze.
No worries @hichamboushaba , tomorrow or next Monday is totally fine! Thanks