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

feat: Add native sqflite indexeddb database #1616

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

krille-chan
Copy link
Contributor

@krille-chan krille-chan commented Nov 17, 2023

Another approach to add former fluffybox directly to
the sdk. So we avoid any
requirement for a new name.
Just name it "MatrixSdkDatabase".

Benchmarks from 22-11-2023

  • FluffyChat latest main in profile mode
  • Flutter 3.16.0
  • Fairphone 5 with Android 13
  • Account on famedly.de Synapse with 169 rooms

HiveCollections (for comparison)

  • Branch: main
  • Initial sync: Benchmark: Process sync -> 10181 ms (3955 items, 2.574209860935525 ms/item)
  • App-start after initial sync:
    • Benchmark: Build database -> 114 ms
    • Benchmark: Get all user device keys from store -> 1431 ms
    • Benchmark: Get room list from store -> 691 ms

Matrix SDK Database (this here! Formerly known as FluffyBox)

  • Branch: krille/add-matrix-sdk-database
  • Initial sync: Benchmark: Process sync -> 5248 ms (3961 items, 1.3249179500126231 ms/item)
  • App-start after initial sync:
    • Benchmark: Build database -> 320 ms
    • Benchmark: Get all user device keys from store -> 17 ms
    • Benchmark: Get room list from store -> 170 ms

Pure SQFlite Database (other approach)

  • Branch: krille/sqflite-database
  • Initial sync: Benchmark: Process sync -> 13808 ms (3961 items, 3.4859883867710173 ms/item)
  • App-start after initial sync:
    • Benchmark: Build database -> 270 ms
    • Benchmark: Get all user device keys from store -> 101 ms
    • Benchmark: Get room list from store -> 323 ms

Notes

All tests have been done with encryption. So we used HiveCipher and SQLCipher.

I assume that Build database is more or less equal in Matrix SDK Database and the pure SQFlite Database and the difference is just luck here. That processing sync is so much slower there might be caused by that the pure sqflite database needs to handle some logic in sql while the Matrix SDK Database does this in Dart and then just puts values in the DB. Just putting key/values and dump a whole table seems to be the fastest with SQFlite.

What is definitely still ugly in any key/value store is our handling of inboundGroupSessions and the aggressive caching. But this can be improved to optimize RAM usage and speed.

@krille-chan
Copy link
Contributor Author

Replaces #1541

@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch from ba8087c to d2a4adb Compare November 17, 2023 13:15
@krille-chan krille-chan changed the base branch from krille/new-fluffybox to main November 17, 2023 13:16
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch from d2a4adb to fbe77a9 Compare November 17, 2023 13:33
@nico-famedly
Copy link
Member

Hm, I feel like having an explicit IndexedDb and SqfliteDb backend would make more sense, instead of building an abstraction just to tear it down. Yes, this would be more maintenance overhead, but it would allow some neat optimizations like cursors into responses.

@krille-chan
Copy link
Contributor Author

Hm, I feel like having an explicit IndexedDb and SqfliteDb backend would make more sense, instead of building an abstraction just to tear it down. Yes, this would be more maintenance overhead, but it would allow some neat optimizations like cursors into responses.

Well we had this discussion in refinement meeting already and here: https://docs.google.com/document/d/1gkGEuoldL0IvFod05a4NTXVY4T4pXsGxL6EwSI41-o0/edit#heading=h.as6xvrbd4ygj I personally think the risk of platform specific bugs is too high. But we can put this again on the agenda on the next refinement

@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch from fbe77a9 to 4241674 Compare November 18, 2023 15:31
@krille-chan krille-chan marked this pull request as draft November 18, 2023 15:33
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch 3 times, most recently from 4623969 to c8a1e59 Compare November 22, 2023 09:02
@krille-chan krille-chan marked this pull request as ready for review November 22, 2023 09:55
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch 2 times, most recently from 492a1c7 to 2b08db3 Compare November 29, 2023 14:11
Copy link
Member

@nico-famedly nico-famedly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the actual database is too long for me to review and I am just assuming it is correct, since you copied it from the old implementation. There are some small suggestions here and there and some questions I have, otherwise this looks good to me.

lib/src/database/indexeddb_box.dart Outdated Show resolved Hide resolved
lib/src/database/indexeddb_box.dart Outdated Show resolved Hide resolved
lib/src/database/indexeddb_box.dart Outdated Show resolved Hide resolved
lib/src/database/indexeddb_box.dart Outdated Show resolved Hide resolved
lib/src/database/indexeddb_box.dart Show resolved Hide resolved
test/box_test.dart Outdated Show resolved Hide resolved
lib/src/database/matrix_sdk_database.dart Show resolved Hide resolved
lib/src/database/matrix_sdk_database.dart Outdated Show resolved Hide resolved
lib/src/database/matrix_sdk_database.dart Outdated Show resolved Hide resolved
lib/src/database/matrix_sdk_database.dart Outdated Show resolved Hide resolved
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch 11 times, most recently from a2397b6 to 67b44ad Compare December 5, 2023 13:11
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch 5 times, most recently from 7050e81 to 341e383 Compare December 6, 2023 10:51
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch from 341e383 to 4e80cc0 Compare December 6, 2023 11:10
fix: Edit last event breaks db

feat: Add native sqflite indexeddb database

feat: Split up preload and nonpreload room state boxes
@krille-chan krille-chan force-pushed the krille/add-matrix-sdk-database branch from 4e80cc0 to 6db019a Compare December 6, 2023 11:12
@krille-chan krille-chan merged commit 67d0a20 into main Dec 6, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants