Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.29k stars 2.72k forks source link

[HIGH] Add SQLite support to web browser and fallback to IndexedDB #34289

Closed muttmuure closed 5 months ago

muttmuure commented 7 months ago

Placeholder tracking issue for adding SQLite support to Chrome, Safari, Edge and FireFox (Not IE)

Also for adding a fallback mechanism to retrieve data from IndexedDB when SQLite fails or has a problem

cc @roryabraham

MIAhmed commented 7 months ago

Below is the way to check if the browser supports SQL Lite or IndexedDB then we can perform the appropriate operations like storing and retrieving data from either SQL Lite or IndexedDB.

// Check for SQLite support
const isSQLiteSupported = 'openDatabase' in window;

// Check for IndexedDB support
const isIndexedDBSupported = 'indexedDB' in window;

if (isSQLiteSupported) {
  // Initialize SQLite database
  // ...
} else if (isIndexedDBSupported) {
  // Initialize IndexedDB
  // ...
} else {
  // Fallback to other storage mechanisms or do an alternative
  console.error("SQLite and IndexedDB are not supported in this browser.");
}
muttmuure commented 7 months ago

Hey @MIAhmed! What's your Slack handle? Would you be interested in creating a proposal for this?

roryabraham commented 7 months ago

Thanks for running with this @muttmuure, I think generally there was consensus that this is a good idea, but let me post a formal proposal first just to get everyone on board. Going to slap a planning label on this and put it on HOLD

melvin-bot[bot] commented 7 months ago

Triggered auto assignment to @lschurr (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

roryabraham commented 7 months ago

Chatting in this thread

adhorodyski commented 7 months ago

Does this still rely on the deprecated WebSQL standard? As far as I'm aware only WASM is now a viable option to use SQL on this platform.

MIAhmed commented 7 months ago

Hey @MIAhmed! What's your Slack handle? Would you be interested in creating a proposal for this? @muttmuure Yes, I am interested in implementing fallback to the indexedDB as I have already implemented that. I do not have Slack access

roryabraham commented 7 months ago

Does this still rely on the deprecated WebSQL standard? As far as I'm aware only WASM is now a viable option to use SQL on this platform.

No, I'm referring to the official SQLite wasm build

roryabraham commented 7 months ago

Sounds like we got consensus to do some exploratory research here and investigate the potential performance benefits.

roryabraham commented 7 months ago

For transparency to our open-source community, we've asked Margelo to begin researching this since they're experienced with working in Onyx and maintain https://github.com/Margelo/react-native-quick-sqlite

chrispader commented 7 months ago

commenting here for assignment! @roryabraham

roryabraham commented 7 months ago

@chrispader having set up wasm support for Encryptify, I wonder if this is something we can implement at the react-native-quick-sqlite layer?

That way, we would be able to leave Onyx mostly unchanged – just check if OPFS is supported, and if it is use the existing quick-sqlite storage provider, if not fall back on IndexedDB.

We could also create a log of grafana graph to see how much the IndexedDB provider is actually used in the wild. That way a year from now there's no guesswork – we can look at how much it's actually used by users and consider dropping it from Onyx to simplify the codebase.

chrispader commented 7 months ago

I did some research today and here's are my initial thoughts: cc @roryabraham @muttmuure @adhorodyski

(Lmk if you have any comments/critic)

Which SQLite + WASM approach to take

As suggested in the documentation of the official library (SQLite WASM) i would go for the approach where we use the extra wrapped worker thread. This way, additionally to (potentially) improved speed we have the benefit of offloading much of the load in Onyx to a different thread.

Reference: Worker + Promise approach

@chrispader having set up wasm support for Encryptify, I wonder if this is something we can implement at the react-native-quick-sqlite layer?

I would definitely add this functionality to react-native-quick-sqlite directly. The library is not yet web-compatible anyway, so this would simply extend the library by adding the whole web part and wouldn't interfere with the native implementation at all.

Onyx integration

That way, we would be able to leave Onyx mostly unchanged – just check if OPFS is supported, and if it is use the existing quick-sqlite storage provider, if not fall back on IndexedDB.

Exactly! As you've suggested, we would then check for availability of OPFS in Onyx and use IndexedDB (idb-keyval provider) as fallback.

Onyx operates async anyways, so this approach wouldn't require any changes in the library.

Relational data approach / Onyx re-design?

From https://expensify.slack.com/archives/C05LX9D6E07/p1704972138886149?thread_ts=1704921473.038539&cid=C05LX9D6E07 by @adhorodyski

one of the benefits is the unification of the query layer - this can potentially really simplify the architecture which is now limited to the least capable platform (key-value on web). Think indexes for example, running migrations, or bringing in an ORM for a type-safe database access.

I'm not sure if i understood your idea 100%, but if it refers to re-designing Onyx to support relational data, i don't think that would be a good idea atm. I wouldn't suggest to rely on any specific storage provider implementation too much, like SQLite on both web and native. Or was the idea to drop Onyx and create a completely re-designed (relational data) storage solution?

I like the idea of improving the overall performance by not trying to convert relational data into key-value data and therefore losing potential of indexing and efficient querying. I just think it wouldn't be a good idea to try achieve this in Onyx...

Imo Onyx is exclusively designed as a key-value store and its big advantage is that it's not dependent on any specific storage layer, instead it can use any underlying storage solution... and we will potentially improve this aspect even more.

I also agree with @tgolen, that as long as we don't completely re-structure our approach to querying and persisting data, i don't think we need react-query right now.

(Correct me if i got any aspect of your suggestion wrong though)

Performance

As @roryabraham already mentioned in this Slack thread the main benefit would definitely be faster speeds and to offload a lot of work to a separate worker thread and therefore unblock the main thread and improve app's performance.

I haven't created any real benchmarks and performance comparisons with IndexedDB yet, but from what i've read it's definitely faster than IndexedDB.

(Asking about what to do next in "Going forward...")

Analytics

We could also create a log of grafana graph to see how much the IndexedDB provider is actually used in the wild. That way a year from now there's no guesswork – we can look at how much it's actually used by users and consider dropping it from Onyx to simplify the codebase.

That definitely makes sense! Do we already use Grafana and is there an account/token i can use?

I could also imagine adding analytics for web and native performance of SQLite. This way we could analyze which kind of devices profit the most from this change and which might have problems.

Going forward...

@roryabraham @tgolen just to make sure i'm not putting too much work into this straight ahead..

Was the idea to create a P/S for this with all the actual problems solved in the app first?

Or do we want to dive straight into the actual implementation in react-native-quick-sqlite? After my initial research, i don't see any functionality of the library that couldn't be realized with the WASM implementation of SQLite.

Also, do we want to create performance benchmarks before or after the actual implementation in the library?

chrispader commented 7 months ago

As for the implementation:

react-native-quick-sqlite is currently only synchronous. Also, the WASM API is limited, though totally sufficient for the use in Onyx/Expensify.

My idea would be to create a separate version inside the react-native-quick-sqlite library, which is

This way, our other users of the library aren't affected and if the WASM implementation of SQLite get's more comprehensive in the future, we can extend the scope of the new version and also provide an additional async version.

Onyx is designed asynchronously anyways, so we can just replace the sync calls to react-native-quick-sqlite with async ones.

roryabraham commented 7 months ago

Thanks @chrispader, here are my thoughts:

Was the idea to create a P/S for this with all the actual problems solved in the app first? ... Or do we want to dive straight into the actual implementation in react-native-quick-sqlite?

Basically, I think we should try to do the minimum amount of work possible to get real benchmarks that show the difference in performance between IndexedDB and SQLite-wasm. It would also be interesting to see how offloading Onyx work to a separate worker thread would help speed up the app, but I don't see how we could investigate that without building out a complete solution.

Once we have benchmarks that show the performance change, we can evaluate whether it's worth pursuing further. I admit it's difficult to write a problem statement for performance improvements because "how fast is fast enough"? I think a major selling point for us though is that the amount of work in Onyx and E/App to make this change is very small – all the work prettymuch is in react-native-quick-sqlite.

react-native-quick-sqlite is currently only synchronous

Maybe for simplicity start with the synchronous SQLite-wasm implementation then? I'd think that the way to approach this would be:

That way there's no breaking changes, and all platforms have both sync and async capabilities, without any confusing configuration, sub-packages, or patches

quinthar commented 7 months ago

Just confirming that we have @danielk1977 and @drhsqlite in this discussion, as they are the literal experts!

danielk1977 commented 7 months ago

I'm here. But @sgbeal is the sqlite/wasm expert.

melvin-bot[bot] commented 7 months ago

📣 @danielk1977! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
sgbeal commented 7 months ago

As suggested in the documentation of the official library (SQLite WASM) i would go for the approach where we use the extra wrapped worker thread. This way, additionally to (potentially) improved speed we have the benefit of offloading much of the load in Onyx to a different thread.

Reference: Worker + Promise approach

If i may interject here, as the person who wrote that documentation and code: The worker1 API is not recommended for any serious work. It has some severe limitations, noted in the above link, e.g. making it work properly with transactions is difficult and it cannot handle nested loops of queries because JS's postMessage() model simply can't support it.

The sqlite/wasm library can, and arguably should, be run in a separate thread without the worker1 API. The most flexible way to use it in a separate thread is to load the library and any directly-db-using client code into the same worker. That gives the client code full access to the sqlite API without the limitations of the worker1 API. It's not only much more comfortable to use that way, but also much more powerful. The one down-side is that the client-side code on the other side of the worker will have to coordinate its own communication do the db-using worker. That also has advantages, however, such as being able to encapsulate all of the SQL in the db-using thread and accessing it via a higher-level postMessage() API from the main thread.

melvin-bot[bot] commented 7 months ago

📣 @sgbeal! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
sgbeal commented 7 months ago

I haven't created any real benchmarks and performance comparisons with IndexedDB yet, but from what i've read it's definitely faster than IndexedDB.

We have never worked with IndexedDB and have no benchmarks comparing it. We do have a wasm port of sqlite3's native-level benchmarking tool, speedtest1, which can be used to get a fairly good idea of its overall speed:

The baseline is an in-memory db:

https://wasm-testing.sqlite.org/speedtest1-worker.html?size=15

Then we have two alternatives for persistence via OPFS, each with their own trade-offs, as documented here:

(That "size" URL parameter can be used to adjust the workload size - note the query counts in the output.)

I/O of the former is significantly slower than the latter but it offers a moderate level of concurrency (multi-tab use), whereas the latter is blazing fast but cannot be accessed from more than one tab/window at a time. Note that "slow" and "fast" are always relative, however, and the former is "fast enough" for many types of applications.

Note that concurrency in OPFS is a pain point - it does not natively support any concurrency without the use of experimental new APIs which were only recently released and are not available outside of Chrome. Our first VFS works around those limitations somewhat but it cannot reliably support more than a few browser tabs visiting the same db without locking-related issues. Our second VFS sacrifices all concurrency support for speed, and a given pool of dbs can only be accessed by a single tab concurrently.

For the sake of transparency, i feel compelled to point out that our wasm build is not the only option. Roy Hashimoto's wa-sqlite project is actively developed, supports multiple storage back-ends (including IndexedDB) via the sqlite VFS API, and is specifically developed for the cutting-edge JS ecosystem. In contrast, we publish plain-vanilla JS and make no use of node.js-based toolchains, which has been a point of contention with some potential users but it's a stance we must stick to for the sake of developer bandwidth and sanity.

chrispader commented 7 months ago

If i may interject here, as the person who wrote that documentation and code: The worker1 API is not recommended for any serious work. It has some severe limitations, noted in the above link, e.g. making it work properly with transactions is difficult and it cannot handle nested loops of queries because JS's postMessage() model simply can't support it.

Ah ok, thanks for the notice! By the "suggested approach in the official documentation" i was mostly referring to the GH Readme where it says "preferred option 🏆). If you suggest to use a differnet, i'd be glad to go with that instead!

The sqlite/wasm library can, and arguably should, be run in a separate thread without the worker1 API. The most flexible way to use it in a separate thread is to load the library and any directly-db-using client code into the same worker. ... The one down-side is that the client-side code on the other side of the worker will have to coordinate its own communication do the db-using worker. That also has advantages, however, such as being able to encapsulate all of the SQL in the db-using thread and accessing it via a higher-level postMessage() API from the main thread.

If i understand you correctly here, this would mean that we can't or shouldn't implement this directly as-planned into react-native-quick-sqlite and indirectly into react-native-onyx, because we also need client-code from Expensify/App to run in the worker thread and directly communicate with the database.

I mean there is probably some hacky way to achieve this with all sorts of callbacks, but that already sounds like callback hell...

(@sgbeal please correct me if anything in this statement is incorrect or if i am mistaken somehow)

cc @roryabraham

chrispader commented 7 months ago

Also JFYI @roryabraham

I'm going to be 100% OOO from 31/01/2024 - 17/03/2024. From what i can see right now, the work on this feature will either take longer than anticipated (from my side) and/or involve much more than just a change to react-native-quick-sqlite.

Therefore, i can finish my research on this and we can figure out the best approach to achieve this feature and then i'd have to hand this task over to another Margelo engineer. Maybe @Szymon20000 or @janicduplessis, since they also both have experience with react-native-onyx and/or react-native-quick-sqlite.

I'm happy to put more work into research, benchmarking and design of implementation until then, but i think the comment from @sgbeal changes a lot about my initial proposal and our initial idea of implementing this.

sgbeal commented 7 months ago

If i understand you correctly here, this would mean that we can't or shouldn't implement this directly as-planned into react-native-quick-sqlite and indirectly into react-native-onyx, because we also need client-code from Expensify/App to run in the worker thread and directly communicate with the database.

My experience with React is literally zero, so i can't speculate on what is or is not feasible with it, but loading both sqlite and your db-side client code into a single thread should not limit what you can do, beyond any limitations which you build in to that abstraction.

Let's say you've got a primary thread (be it the main window thread or a worker - that's largely irrelevant for sqlite), then load the library and your db-side code into its own shared thread. That gives your db-side code full access to the sqlite API (both the C-style API and a higher-level OO APIs), with no limitations on what it can do vis a vis the sqlite API. Your primary thread could then postMessage() requests to your client-level code in the db thread, and you can react to that however you like, within the limitations of the postMessage() protocol (e.g., iterating over nested loops of queries is not possible with that approach, and transactions are tricky to get right).

Your primary thread can post a message like:

postMessage({type: 'getDataForCustomer', customerId: 123456 })

and your db thread will have full access to the sqlite API, including custom SQL functions, to fetch that data, reformat it into whatever form you want the primary thread to have, and return it via a response postMessage(). The supplied worker1 API and its "promiser" proxy simplify that interaction somewhat, but its model of working introduces several significant limitations, such as:

Plus the SQL will all have to be part of your primary thread, rather than separated neatly into the db-using thread.

Even so, it would not require a significant development effort to write your own custom worker1-like proxy which is specific to your APIs and moves all of the db-related logic into the db worker, which your primary thread then remote-controls via a custom-fit API. That would enable you to eliminate the above limitations because they would not apply to that hypothetical new remote-controlled worker.

I mean there is probably some hacky way to achieve this with all sorts of callbacks, but that already sounds like callback hell...

Working via postMessage() always has a bit of callback hell involved, even through the callbacks are effectively encapsulated in the onmessage handler. The worker1 promiser proxy effectively uplifts those callbacks from onmessage into something more recognizable as callback hell, but it's still cross-thread callback hell, and the cross-thread aspect of it introduces the above-mentioned significant usage limitations. Those APIs were created as proxies for simple use cases, and suffice for certain types of applications, but your apps would almost certainly outgrow those capabilities within days.

You won't be able to avoid postMessage() altogether unless you have sqlite in your main app thread, but OPFS is not available when sqlite is run that way. We can, however, simplify your use of postMessage() by housing your db-using code and sqlite in the same worker thread and remote-controlling that combination from your primary thread.

I'm happy to put more work into research, benchmarking and design of implementation until then, but i think the comment from @sgbeal changes a lot about my initial proposal and our initial idea of implementing this.

Likewise, we're happy to work with you to recommend a best-fit approach, with the caveat that our collective React know-how and ambitions are literally zero. Since the JS ecosystems evolves so rapidly, beyond our tiny team's ability to keep up with or sensibly claim to support, we very specifically provide only plain-vanilla JS which we then expect interested parties to integrate into their favorite framework as they see fit. We can't assist much with the framework-specific details but are happy to assist with both the integration and how to make use of the JS APIs.

We're available just about any time for video calls, if you'd like to talk through this.

adhorodyski commented 7 months ago

Relational data approach / Onyx re-design?

From https://expensify.slack.com/archives/C05LX9D6E07/p1704972138886149?thread_ts=1704921473.038539&cid=C05LX9D6E07 by @adhorodyski

one of the benefits is the unification of the query layer - this can potentially really simplify the architecture which is now limited to the least capable platform (key-value on web). Think indexes for example, running migrations, or bringing in an ORM for a type-safe database access.

I'm not sure if i understood your idea 100%, but if it refers to re-designing Onyx to support relational data, i don't think that would be a good idea atm. I wouldn't suggest to rely on any specific storage provider implementation too much, like SQLite on both web and native. Or was the idea to drop Onyx and create a completely re-designed (relational data) storage solution?

I like the idea of improving the overall performance by not trying to convert relational data into key-value data and therefore losing potential of indexing and efficient querying. I just think it wouldn't be a good idea to try achieve this in Onyx...

Imo Onyx is exclusively designed as a key-value store and its big advantage is that it's not dependent on any specific storage layer, instead it can use any underlying storage solution... and we will potentially improve this aspect even more.

I also agree with @tgolen, that as long as we don't completely re-structure our approach to querying and persisting data, i don't think we need react-query right now.

(Correct me if i got any aspect of your suggestion wrong though)

also @cc @dsilva as you might be interested in this:

Hey @chrispader, you totally got this right and I wanted to shed some light onto the React side of things and how SQLite in the long run can be integrated with the UI layer, not Onyx-related. My idea is definitely not restructuring Onyx and I agree, react-query doesn't fit nowhere in there currently :)

What I rather wanted to propose is establishing a long-term vision for a new solution that is really scalable and future proof for the app's performance at any scale. This includes relying on the relational data structures and the power of SQL as well as the latest concurrency features available in React.

The problem

Currently, we rely on the key-value (KV) architecture even for mobile devices (running SQLite) which forces us to calculate all the bits of information on the same thread that is responsible for handling user interactions. No matter where we got the data from (be it indexedb or an sqlite table), as the scale increases, we have to add up another iterations and use more of the device resources to handle the workload. For reference, please see the below screenshot showing how this looks like for an Android device on the JS thread, with Heavy account when sending a message (one of the core metrics):

defaultProductionHermesTrace

As we can see, most of the time (here ~15 seconds) is being spent on resorting list items, completely blocking user interactions, and it’s like this by design - with KV we can do all the tricks, but every filter operation, fields recalculation and all the heavy lifting still sits on the JS engine.

This can be optimised, but only to a certain point at which we won't be able to squeeze anything more out of it without sacrificing a few seconds for the most basic operations. I’m sure this is nowhere we want to be a few years from now.

What's the most important, this pattern spreads across our core metrics (App startup, Opening up a report, Sending a message), making it really hard (if not impossible at some point) to hit the performance targets as we scale.

Solution

Even though we have SQLite, we don’t currently use nearly any of its features. We utilise it as a KV.

For the long term, as SQLite becomes available also for the web platform, I propose utilising the tool at hand to its full potential, enabling us to offload all the data related calculations in the App to the database through SQL.

Efficient querying with filters, joining tables, sorting (ordering), indexing or running migrations can all run outside of the JS thread within milliseconds.

An example from before running on the database through ORDER BY should take only some milliseconds and be instantly available to the UI.

Moreover, all of this can operate with some really minimal tooling around as React aims to fully support Promises with the concurrent features - and even the most complex query running through a database driver is just a Promise. This means that we can replace Onyx with a really thin and efficient communication layer between the UI and DB and also provide a good developer experience - will less code to write, easy to follow patterns and minimalistic abstractions.

There are also a few projects which currently try to address these problems in a similar way:

https://www.evolu.dev/

I really wanted to open this up for some feedback, as I’ve heard there’s an interest in utilising SQLite more as we get the web support in place.

An offline-first application should be no slower than tens of milliseconds per operation basically no matter the scale - and that’s what we can definitely aim for.

quinthar commented 7 months ago

We are huge huge fans of SQLite. Can you outline a strategy for how to incrementally migrate Onyx to use more SQLite functionality, without needing to rewrite everything or lose momentum? You've mentioned that grouping, sorting, and filtering are all great features of a relational database, and that doing these after the fact in the application layer is super slow. Maybe you could pick a couple scenarios that are particularly slow and suggest how in theory we could move those slow operations into the SQLite layer, without fundamentally breaking the Onyx API layer?

quinthar commented 7 months ago

Escalating this conversation to Slack to make it go faster: https://expensify.slack.com/archives/C01GTK53T8Q/p1706933222963169

roryabraham commented 7 months ago

Left a proposal in the slack thread to try and gain consensus on next steps here.

drhsqlite commented 7 months ago

The link takes me to the "Glitch" page (screen-capture attached). Can you email a copy?[glitch.gif]

-- D. Richard Hipp @.***

On Wednesday, February 7th, 2024 at 12:35 AM, Rory Abraham @.***> wrote:

Left a proposal in the slack thread to try and gain consensus on next steps here.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

melvin-bot[bot] commented 7 months ago

📣 @drhsqlite! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
roryabraham commented 7 months ago

Ah, @drhsqlite that's a link to our #expensify-open-source slack room. We can invite you if you'd like, but the proposal is not specific with regards to the implementation, just high-level next steps

image

Most of the links in that comment are to internal slack rooms, but you can see some previous benchmark results comparing two competing IndexedDB storage solutions here

hannojg commented 7 months ago

Yes, happy to add it to the existing performance benchmark tool and get us some numbers!

kirillzyusko commented 7 months ago

Hey, I am helping with implementing the performance benchmark, please assign me as well. Thanks!

kirillzyusko commented 6 months ago

Hello everyone 👋

These are preliminary benchmark results (all values are in ms):

Operation Baseline (mock storage) IDB (current) Local forge SQLite (main) SQLite (worker) SQLite (worker) - sql queries execution time
Single set 115.9 157.8 170.1 155.4 193.2 21.9
Get collection 10.39 9.46 10.66 9.55 10.39 9.21
Merge collection 365.7 619.8 61338.8 542.7 600.8 229.6
Single merge 114.7 185.6 192 192.4 311.2 124.1
Clear 99.08 982.7 681 166.4 612 506.2

The agenda is:

All metrics were captured with depth=1, length=35, items=10_000 and all operations were performed 10x times and then average value was calculated (sum / 10).

The working branch is: https://github.com/margelo/react-native-onyx/tree/feat/sqlite-wasm

As I can see single merge and single set operations are slower comparing to IDB - I think it's mainly due to the fact that we are passing large JSON objects into postMessage and data serialization steals some CPU time. I'll investigate it further whether it's possible to optimize it or not 👀

[!NOTE] I've also added a column where I measured pure SQL queries execution (without additional pass to/from worker).

tgolen commented 6 months ago

Thanks. Are those values in milliseconds?

From those preliminary results, it doesn't look like there are any significant gains, and maybe even some losses. JSON serialization is the worst :rage4:

Hopefully, there is a way to optimize it.

kirillzyusko commented 6 months ago

Are those values in milliseconds?

Yes, in milliseconds.

Hopefully, there is a way to optimize it.

I'll try to find possible ways 👀

sgbeal commented 6 months ago

SQLite (worker) - SQLiteWASM running on a worker thread (with OPFS)

Note that there are two distinct OPFS VFSes. One of them trades some level of concurrency (multi-tab access) for a significant cost in speed and the second forgoes all concurrency for corresponding speed gains (it's typically 2-3x faster than the other one, depending on the workload, and is never slower). Those VFSes can be run through a wasm build of sqlite's "speedtest1" benchmark, the project's own I/O-centric benchmarking tool, via:

https://wasm-testing.sqlite.org/

See the "speedtest1" links around the center of the page for the various options, including in-memory, "opfs" (the first OPFS VFS), "opfs-sahpool" (the second OPFS VFS), and "kvvfs" (which can use sessionStorage and localStorage for small amounts of persistence in the main thread).

Chrome v121 has a new, experimental, still-up-for-change feature which should permit us to add concurrency support to the second (faster) VFS, but (A) that's still a big question mark and (B) it's currently only in bleeding-edge Chrome versions (and subject to change).

The two current OPFS VFSes are covered at:

https://sqlite.org/wasm/doc/trunk/persistence.md#opfs

kirillzyusko commented 6 months ago

Thanks for pointing out @sgbeal

I used new sqlite3.oo1.OpfsDb(/db.sqlite3); construction, I think by default it resolves to vfs=opfs (i. e. lower speed, concurrency support), right?

Anyway I updated table (I added benchmark only for SQL queries execution) and in this case SQLite shows the fastest results. So I think the problem here is in data serialization between threads. I'm investigating whether it can be improved somehow 👀

sgbeal commented 6 months ago

I think by default it resolves to vfs=opfs (i. e. lower speed, concurrency support), right?

Correct. That constructor is hard-coded to that particular VFS.

So I think the problem here is in data serialization between threads.

Thread-related waiting is definitely our single biggest performance hit for the "opfs" VFS, but that seems to be (based on unscientific experiments) related to the underlying browser-side mutexes and the related timers, rather than the data serialization itself (see below).

The new/experimental "readwrite-unsafe" mode which Chrome has as of v121 is expected to help us in that regard, but the fact that it's still Chrome-only and subject to change has kept us from experimenting with it. Strictly hypothetically, that feature can be used to extend the much faster 2nd OPFS VFS to support concurrency, which would be a huge win.

Regarding the byte-level serialization: rather than postMessage() data back and forth between the VFS-specific thread and the library thread, the VFS thread communicates with the library's thread over a SharedArrayBuffer, so there is no extraneous copying going on between those threads. The two threads use Atomics.notify() to make each other aware of new data in the buffer. The communication between the library and the VFS always happens in chunks equal to the db page size, so the VFS can (and does) reserve a fixed-sized SharedArrayBuffer for its communication with the library. The major performance hit in all of that is us waiting on the cross-thread communication between Atomics.notify() and Atomics.wait(), all of which happens in black-box browser internals.

danieldoglas commented 6 months ago

Quick noob question:

based on the speed test in https://wasm-testing.sqlite.org/speedtest1.html, if instead of storing JSONs we had a few structured tables for data that will be abundant and we have a format that is mostly defined (reportActions, transactions) would those operations be faster?

kirillzyusko commented 6 months ago

if instead of storing JSONs we had a few structured tables for data that will be abundant and we have a format that is mostly defined (reportActions, transactions) would those operations be faster?

@danieldoglas SQLite is also fast with plain JSON messages. The main bottleneck right now is a communication between threads (main <--> worker).

kirillzyusko commented 6 months ago

Regarding the byte-level serialization: rather than postMessage() data back and forth between the VFS-specific thread and the library thread, the VFS thread communicates with the library's thread over a SharedArrayBuffer, so there is no extraneous copying going on between those threads. The two threads use Atomics.notify() to make each other aware of new data in the buffer. The communication between the library and the VFS always happens in chunks equal to the db page size, so the VFS can (and does) reserve a fixed-sized SharedArrayBuffer for its communication with the library. The major performance hit in all of that is us waiting on the cross-thread communication between Atomics.notify() and Atomics.wait(), all of which happens in black-box browser internals.

@sgbeal so you are saying, that usage SharedArrayBuffer should make communication faster? I thought about experimenting with it, but still kind of afraid, that writing/reading from SharedArrayBuffer can be a bottleneck 🤔 But I'll try to see how it goes! 👀

sgbeal commented 6 months ago

based on the speed test in https://wasm-testing.sqlite.org/speedtest1.html, if instead of storing JSONs we had a few structured tables for data that will be abundant and we have a format that is mostly defined (reportActions, transactions) would those operations be faster?

That's really anyone's guess and mine would be mere speculation. That will, at least in part, depend on whether those objects span db pages, because...

@sgbeal so you are saying, that usage SharedArrayBuffer should make communication faster?

Faster compared to (say) postMessage() because it eliminates an extra copy which might otherwise be necessary[^1]. For example, when the library reads from storage, it passes a destination byte array to the VFS. In this particular case, the library and the VFS-related I/O live in different threads but they communicate via the same SharedArrayBuffer, so the I/O itself is performed directly on that shared buffer. The coordination/synchronization of the communication between those two threads, irrespective of the actual I/O, is where the biggest single performance hit comes from. The process goes something like:

The same happens for writing: the library thread populates the SAB, tells the I/O thread that data is available, and then waits on the I/O thread's response to that write. Thanks to the SAB, the I/O thread does not need to make a separate copy of that data before passing it on to the OPFS layer.

Does that answer your question?

[^1]: postMessage() can move, rather than copy, buffers between threads, but in our case doing so is more expensive/slower than an Atomics.wait/notify()-based construct because we have to either create a new buffer for each operation or coordinate moving of a single buffer back and forth between threads. Our very first prototype used postMessage() for the cross-thread communication but it didn't perform as well.

kirillzyusko commented 6 months ago

Does that answer your question?

Yes, thank you 🙏

We've investigated SharedArrayBuffer usage, but since it operates on bytes level, we anyway need to serialize/deserialize a data and here we will loose our performance. I. e. if we do JSON.stringify to pass a data to worker, then here we are wasting a lot of CPU resources (but if there is another optimal way of converting JS object into SharedArrayBuffer - let me know).

We also looked at the performance when we have no serialisation/deserialisation on the worker thread API, but assuming that all the data layer code would operate in the worker thread directly (ie. API calls happening there and onyx code lives there as well). For that case we have these results:

Operation SQLite (worker) - sql queries execution time
Single set 21.9
Get collection 9.21
Merge collection 229.6
Single merge 124.1
Clear 506.2

I. e. if there is no communication between threads, then SQLiteWASM is an absolute winner 💪 (I also added these numbers to main comparison table).

Let me know what do you about the results and further direction 😊

sgbeal commented 6 months ago

We've investigated SharedArrayBuffer usage, but since it operates on bytes level, we anyway need to serialize/deserialize a data and here we will loose our performance. I. e. if we do JSON.stringify to pass a data to worker, then here we are wasting a lot of CPU resources (but if there is another optimal way of converting JS object into SharedArrayBuffer - let me know).

AFAIK there is no more optimal approach for general-purpose or high-level software. We internally use a fit-to-purpose serialization approach which has proven to be faster, but it's in no way flexible. It depends, for example, on having a fixed number of fixed-size values, which isn't practical for anything but the most specialized of use cases.

Let me know what do you about the results and further direction

Unfortunately, my lack of background with your apps, infrastructure, and use cases leaves me unable to say terribly much which is likely to be of use.

If you like, we can do a video call and, with some hand-holding from you, perhaps i'll be able to offer some concrete suggestions and/or provide guidance on getting the most out of sqlite's JS/WASM APIs. Just send a meeting link and time to support at sqlite org and i can make the time slot work.

kirillzyusko commented 6 months ago

@sgbeal I sent an invitation to your email (took it from github profile, sorry, didn't find your contact on support page)

sgbeal commented 6 months ago

I sent an invitation to your email (took it from github profile,

Got it. See you on Friday!

kirillzyusko commented 6 months ago

We had a meeting with @sgbeal, key notes are:

After a discussion some options with @sgbeal we came to next conclusions:

sgbeal commented 6 months ago

VFS-2 can be used with concurrency, but it's supported only in latest Chrome browser versions

To clarify: that capability is currently hypothetical. The new Chrome feature might allow us to add concurrency to that VFS, but we won't know for sure until we're able to experiment with it. Even if we do that, however, that OPFS feature is experimental and currently only in Chrome, so it may still change in incompatible ways. We have no information about whether the other browsers plan to adopt that feature and we are not keen on the idea of releasing a Chrome-only VFS.

roryabraham commented 6 months ago

most recent discussion here