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

MWS: Add support for node-sqlite-wasm alongside better-sqlite3 #7996

Merged
merged 25 commits into from
Feb 22, 2024

Conversation

Jermolene
Copy link
Member

Introduction

This PR targets the the Multi Wiki Server PR. It swaps the SQLite implementation from better-sqlite3 to node-sqlite3-wasm.

The difference is that better-sqlite3 consists of JavaScript bindings around a copy of the SQLite3 engine that has been compiled to native code while node-sqlite3-wasm is an adapation of the official SQLite3 Wasm distribution that has been modified to work on Node.js.

At the moment, this change introduces a very significant 100x performance degradation. The measured time for loading the initial bags and recipes is:

  • better-sqlite3 - 0.41s
  • node-sqlite3-wasm - 38.8s

The performance degradation needs to be analysed and addressed before we can consider merging this PR. One positive observation is that the performance degradation goes away when using a memory database, so there's a good chance that file I/O can be optimised further.

Note that this PR currently includes switching to a memory based database to make testing easier.

Why switch to SQLite3 Wasm?

It fits with our long term plans of using the same SQLite3 engine both in the browser and under Node.js. The advantage is that it allows TiddlyWiki to continue to be self contained, without requiring a complex installation and build step.

More immediately, I have run into many problems getting better-sqlite3 running correctly on an Azure Web Service which are all due to binary compatibility issues. It's an area in which I have no expertise and I find myself constantly confused, particularly given the long turnaround times for making changes in cloud environments like Azure. If I am having difficulties with this then I think there's a good chance that these problems will impact ordinary users of TiddlyWiki as well. (The project where I have been exploring running TiddlyWiki on Azure can be seen at https://github.com/Jermolene/tiddlywiki-azure-ci).

In contrast, the Wasm implementation just works on Azure without any fuss and bother, and offers the promise of a single codebase for the browser and the server.

Questions

  • Can we address the performance degradation?
  • Is it worth making the code switchable to work with either better-sqlite3 or node-sqlite3-wasm? There are only very minor differences between them, and MWS already has the necessary abstractions to allow the differences to be hidden away in one place

Appreciation of node-sqlite3-wasm

I'd like to mention @tndrle to congratulate them on a nicely conceived and very neatly executed project. There was very little work involved in switching from better-sqlite3 to node-sqlite3-wasm (the commit is here), and it's been a pleasure to work with it.

I hope that the SQLite team themselves might eventually address the Node.js gap themselves, but in the meantime @tndrle's work is much appreciated.

Seems to be slower, but might make cloud deployments easier by not having any binary dependencies
We will make this configurable
Gives a 20% reduction in startup time on my machine
Copy link

vercel bot commented Feb 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ❌ Failed (Inspect) Feb 22, 2024 11:57am

@Jermolene Jermolene changed the base branch from master to multi-wiki-support February 21, 2024 21:04
I am not intending to make this a long term feature. We will choose one engine and stick with it until we choose to change to another.
https://github.com/tndrle/node-sqlite3-wasm doesn't have transaction support so I've tried to implement it using SQL statements directly.

@hoelzro do you think this is right? Should we be rolling back the transaction in the finally clause? It would be nice to have tests in this area...

I looked at better-sqlite3's implementation - https://github.com/WiseLibs/better-sqlite3/blob/master/lib/methods/transaction.js
@Jermolene
Copy link
Member Author

I've now added support for both better-sqlite3 and node-sqlite3-wasm at the same time. Switching between them requires a change to the default in $:/plugins/tiddlywiki/multiwikiserver/sql-tiddler-database.js, and is not something that I intend to offer as a feature long term.

Even with transactions, node-sqlite3-wasm is about 10 times slower than better-sqlite3, which is not encouraging. I had hoped that @hoelzro's transaction support would help but it appears not.

Anyhow, I think this is ready to merge. We can continue to experiment with the choice of engine, and explore improvements to node-sqlite3-wam

@Jermolene Jermolene merged commit 3fca823 into multi-wiki-support Feb 22, 2024
2 of 4 checks passed
@Jermolene Jermolene changed the title MWS: Switching SQLite3 engine to Wasm instead of compiled C code MWS: Add support for node-sqlite-wasm alongside better-sqlite3 Feb 22, 2024
@Jermolene Jermolene deleted the multi-wiki-support-wasm branch February 24, 2024 09:35
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