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

replace db_connection with rw lock #255

Closed
wants to merge 2 commits into from

Conversation

ruslandoga
Copy link
Contributor

@ruslandoga ruslandoga commented Jul 19, 2023

This PR begins to explore RW-lock approach suggested in #192 (comment)

Here's what I plan to try next:

  • should Exqlite.RWConnection.begin_transation(conn) or Exqlite.RWConnection.transaction(fun) be supported?
  • should gen_statem be used?
  • should there be a cache for prepared statements?

@josevalim
Copy link

should Exqlite.RWConnection.begin_transation(conn) or Exqlite.RWConnection.transaction(fun) be supported?

IMO, no transaction helpers necessary, unless it is required later by EctoSQLite3.

should gen_statem be used?

Up to you :)

should there be a cache for prepared statements?

I would postpone until there is a need, to keep the first PR small.

Copy link
Member

Choose a reason for hiding this comment

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

This module could be deleted and ignored until you are completely done. I don't know of anyone using this module directly.

@warmwaffles
Copy link
Member

should there be a cache for prepared statements?

I would postpone until there is a need, to keep the first PR small.

I remember starting to cache the prepared statements, but then bailed on it due to the complexity of it. I figured since this is all in the NIF it was better to just create it on the fly and then let the garbage collector sweep it away once done.

@ruslandoga
Copy link
Contributor Author

Closing this PR as I think the processes can be moved to ecto_sqlite3 and exqlite would be just a NIF. I'm not 100% sure, but I'm exploring this approach in https://github.com/ruslandoga/sqlite.ex and https://github.com/ruslandoga/ecto_sqlite which a re basically copy-pastes of exqlite/ecto_sqlite3 with some changes here and there (and different project / namespace makes benchmarks easier).

@ruslandoga ruslandoga closed this Oct 9, 2023
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.

3 participants