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

Add support for precompiled statements #704

Open
nikitin-da opened this issue Oct 27, 2016 · 5 comments
Open

Add support for precompiled statements #704

nikitin-da opened this issue Oct 27, 2016 · 5 comments

Comments

@nikitin-da
Copy link
Collaborator

  1. I think it will be helpful to have in LowLevel getter for underlying sqLiteOpenHelper.
    The same thing has been already implemented in StorIOContentResolver.LowLevel and it can solve problems with such particular thinks like precompiled statements if user will need it.
  2. To complete support for prepared statements I suggest to add methods to StorIOSqlite like compiledExecute, compiledUpdateDelete, compiledInsert that will take a special type of resolver to bind arguments.
    @artem-zinnatullin @karlicoss @thevery any thought?
    Refs Add info about performance in the README #427
@artem-zinnatullin
Copy link
Member

Hm. I was thinking about different implementation: on the Query + StorIOSQLite level.

Since our Queries are immutable, we can actually create precompiled statement per query (and as ThreadLocals if needed) and store them as as fixed size LIFO map in SQLiteOpenHelper so if we have precompiled statement for Query — use it, otherwise — precompile and use it!

I don't want to add any new public API for this feature if possible.

Does it sound ok? Please cc Stas, also known as "Stas — DB EXPERT", here

@nikitin-da
Copy link
Collaborator Author

Nice idea!
But should we add an option to enable/disable such optimization per operation, per StorIOSqlite?
Could it happen that precompile + single bind will be less efficient then single operation execute?
LIFO map in SQLiteOpenHelper do you mean LRU cache in StorIOSqlite?
DB Expert, @StanisVS, we need your advice)

@artem-zinnatullin
Copy link
Member

Well, we can try implement that and then decide whether it should be option
or not!

On 1 Nov 2016 9:01 am, "Dmitrii Nikitin" [email protected] wrote:

Nice idea!
But should we add an option to enable/disable such optimization per
operation, per StorIOSqlite?
Could it happen that precompile + single bind will be less efficient then
single operation execute?
LIFO map in SQLiteOpenHelper do you mean LRU cache in StorIOSqlite?
DB Expert, @StanisVS https://github.com/StanisVS, we need your advice)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#704 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA7B3DO5ECql1vvTy_6vLfV_TYNCL051ks5q5tXSgaJpZM4KiCtA
.

@skrzyneckik
Copy link
Contributor

@nikitin-da

Could it happen that precompile + single bind will be less efficient then single operation execute?

As I mentioned in #427 SQLiteDatabase.insertWithOnConflict which is used by StorIO under the hood, creates SQLiteStatement everytime when new data is inserted into db, so if we would reuse it, I don't see where we could loose performance for single executed operations - precompiled statement is created anyway.

Btw I like @artem-zinnatullin idea with LIFO map in SQLiteOpenHelper. I know that there are some concerns about it how this feature can affect whole library, so I would like to propose following approach: add this feature to v1.12.0 as an experimental one (similar approach as new features are introduces to RxJava library), which could be enabled by users who looks for performance boost and it could be turned on by default for later versions. What do you think?

@artem-zinnatullin
Copy link
Member

add this feature to v1.12.0 as an experimental one (similar approach as new features are introduces to RxJava library), which could be enabled by users who looks for performance boost and it could be turned on by default for later versions. What do you think?

SGTM, @nikitin-da you'll be able to test it in Yandex.Mail since it uses DB all the time and see if it increases performance or not!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants