Skip to content
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

Avoid JSON deserialization in common trivial cases #11251

Merged
merged 2 commits into from
Jan 17, 2023

Conversation

starypatyk
Copy link
Contributor

This is a very simple approach to reduce time spent on JSON deserialization when reading file data from the local DB - much simpler than originally proposed in #11181.

I have noticed that in my tests, in more than 90% of the cases, the JSON data retrieved from the database is:

  • sharees - "[]"
  • metadataSize - "null"

The proposed changes handle these common trivial cases without invoking the JSON parser.

With this change and #11227 I have observed that the time spent inside FileDataStorageManager::createFileInstance method is less than 10% of the overall time of FileDataStorageManager::getFolderContent. Remaining 90% is in fileDao.getFolderContent - as expected.

@starypatyk
Copy link
Contributor Author

starypatyk commented Jan 14, 2023

I have looked at the failing tests:

@AlvaroBrey
Copy link
Member

I have looked at the failing tests:

Let's rebase the branch to have the tests run again

@AlvaroBrey
Copy link
Member

/rebase

Signed-off-by: Dariusz Olszewski <starypatyk@users.noreply.github.com>
Signed-off-by: Dariusz Olszewski <starypatyk@users.noreply.github.com>
@github-actions
Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/11251.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@github-actions
Copy link

Codacy

Lint

TypemasterPR
Warnings7676
Errors00

SpotBugs

CategoryBaseNew
Bad practice2727
Correctness4444
Dodgy code335335
Internationalization99
Multithreaded correctness99
Performance5858
Security1818
Total500500

@codecov
Copy link

codecov bot commented Jan 15, 2023

Codecov Report

Merging #11251 (49cccd1) into master (20c22eb) will increase coverage by 0.08%.
The diff coverage is 50.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #11251      +/-   ##
============================================
+ Coverage     31.30%   31.38%   +0.08%     
- Complexity     3351     3368      +17     
============================================
  Files           566      566              
  Lines         41584    41586       +2     
  Branches       5629     5631       +2     
============================================
+ Hits          13016    13052      +36     
+ Misses        26624    26587      -37     
- Partials       1944     1947       +3     
Impacted Files Coverage Δ
...loud/android/datamodel/FileDataStorageManager.java 61.95% <50.00%> (-0.03%) ⬇️
...cloud/android/ui/activity/SyncedFoldersActivity.kt 20.53% <0.00%> (-0.45%) ⬇️
...wncloud/android/providers/FileContentProvider.java 73.45% <0.00%> (-0.31%) ⬇️
...cloud/android/ui/activity/FileDisplayActivity.java 27.08% <0.00%> (-0.09%) ⬇️
.../java/com/owncloud/android/utils/DisplayUtils.java 37.10% <0.00%> (+0.28%) ⬆️
...loud/android/datamodel/ThumbnailsCacheManager.java 40.89% <0.00%> (+0.47%) ⬆️
...d/android/operations/SynchronizeFileOperation.java 39.36% <0.00%> (+14.89%) ⬆️
.../java/com/nextcloud/client/jobs/OfflineSyncWork.kt 53.12% <0.00%> (+32.81%) ⬆️

@AlvaroBrey
Copy link
Member

Great idea, @starypatyk ! Many thanks!

@AlvaroBrey AlvaroBrey merged commit 59a13e0 into master Jan 17, 2023
@delete-merged-branch delete-merged-branch bot deleted the chore/json-common-cases branch January 17, 2023 14:15
@AlvaroBrey AlvaroBrey added this to the Nextcloud App 3.24.0 milestone Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review performance: general/non-specific lag, ANR, etc and rarer exceptions/errors that don't have their own labels technical debt ui/ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants