The Wayback Machine - https://web.archive.org/web/20210124104935/https://github.com/sql-js/sql.js/pull/400
Skip to content

/ sql.js

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

Jump to bottom

Export improvements #400

Draft
wants to merge 1 commit into
base: master
from
Draft

Export improvements #400

wants to merge 1 commit into from

Conversation

@seidtgeist
Copy link
Contributor

@seidtgeist seidtgeist commented May 28, 2020 •
edited

Based on #302 I’ve been trying to improve export, and this has lead me on a path™. Have you considered using one of these options?

  1. VACUUM INTO a temporary file that’s read, unlinked and returned
  2. Using sqlite’s Online Backup API to create a copy that’s returned
  3. Duplicating and returning the working memory via deserialize

In this PR, I’m giving (1) a shot. I’m very interested to hear what you have to say. Ultimately, option (2) might be better suited for a generic export function since it doesn’t incur a VACUUM, and I will most likely give that a try as well.

On a related note: Since the current filesystem is MEMFS, have you considered just using an in-memory database instead? Right now, sqlite believes it’s writing to a persistent filesystem, but MEMFS is backed by memory. Since everything’s happening in-memory anyway, why not use sqlite’s native memory-backed mechanics and drop the MEMFS indirection along the way?

I have no experience with IDBFS (#397), and boy would I prefer if we had something like NativeIO.

Some notes to self:

  • Add test for #159
  • Test using bound statements after export
  • Test using JS function after export
@seidtgeist
Use VACUUM INTO tempfile for export
Loading status checks…
90ff568
@seidtgeist seidtgeist changed the title Persistence improvements Export improvements May 28, 2020
@seidtgeist
Copy link
Contributor Author

@seidtgeist seidtgeist commented May 28, 2020 •
edited

Regarding the test failure:

  • Only the sql-asm.js target fails, all other builds (wasm, asm.js debug, workers) pass
  • It’s breaking because getTempRet0 is undefined when called
  • AFAIK getTempRet0 should be provided by emcc since it’s being called

Looking through emscripten issues it looks like there have been various fun moments with getTempRet0/setTempRet0 over the years 😅
@lovasoa
Copy link
Member

@lovasoa lovasoa commented May 28, 2020 •
edited

The issue55 test is the one in particular that tests this part of the code, I would like to make sure it passes before merging.

Since the current filesystem is MEMFS, have you considered just using an in-memory database instead?

👍 This seems like the most promising and logical thing to me. This should allow us to build with -s FILESYSTEM=0, and hopefully reduce the size of the distributed bundle, without loosing any functionality, just using sqlite3_serialize and sqlite3_deserialize.
@seidtgeist
Copy link
Contributor Author

@seidtgeist seidtgeist commented May 28, 2020

The issue55 test is the one in particular that tests this part of the code, I would like to make sure it passes before merging.

Yeah, I’m trying to figure out how to make emscripten include env.getTempRet0 in the sql-asm.js target output. If I copy the implementation from one of the other files it passes.

Also, VACUUM can take some time and possibly lock up UI or workers. Do you have any concerns with changing the implementation in such a way?

👍 This seems like the most promising and logical thing to me. This should allow us to build with -s FILESYSTEM=0, and hopefully reduce the size of the distributed bundle, without loosing any functionality, just using sqlite3_serialize and sqlite3_deserialize.

I’ll give this a try, too. Having way too much fun with things!
@seidtgeist
Copy link
Contributor Author

@seidtgeist seidtgeist commented May 28, 2020

@kripken Since this was/is your baby, do you have any opinions on exporting and in-memory vs filesystem? And if you have any getTempRet0 tricks up your sleeve... 😜
@kripken
Copy link
Collaborator

@kripken kripken commented May 28, 2020

I think going directly to memory might be better than using a filesystem, yeah. Could be much faster to just malloc some room and use that.

For tempRet0, if you need it you can export it to make sure it exists

-s "EXTRA_EXPORTED_RUNTIME_METHODS=['getTempRet0']"
@Taytay
Copy link
Contributor

@Taytay Taytay commented Jul 16, 2020

For what it's worth, I would love to avoid the overhead of the virtual FS and just tell SQLite that it's writing to memory. Thank you for exploring this @seidtgeist!
@lovasoa lovasoa force-pushed the sql-js:master branch from 351ffdd to 6e7ec33 Oct 11, 2020
@cepm-nate cepm-nate mentioned this pull request Dec 7, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@seidtgeist @lovasoa @kripken @Taytay