-
Notifications
You must be signed in to change notification settings - Fork 6
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
Significant performance improvements and code cleanup #20
Open
vdukhovni
wants to merge
8
commits into
hdbc:master
Choose a base branch
from
vdukhovni:performance-and-cleanup
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Drop "old-locale" dependency and tests of conversions from obsolete "old-time" types. Added stack.yaml, for 6.35. Newer LTS versions can't build the tests due to version caps on template-haskell in dependencies. Compilation without the tests dependencies now works through at least LTS-9.5 and even stackage-nightly-2017-09-20.
We try to finalize it after failed calls, and, though the SQLite3 documentation promises that it will be initialized to NULL on error, this is liable to elicit compiler warnings, and is not "obviously" correct.
- Missing type signatures - Shadowed names - Unused names - Ignored results - Incomplete pattern matches - Missing modules in cabal file * Fixed all warnings reported by GHC 7 in code and tests * Fixed all warnings reported by GHC 8 in the code, tests don't yet build due to version conflicts in dependencies.
This allows prepared statements to stay open, so that one can avoid the penalty of recompiling them each time. Applications that disable auto finish MUST finish() *every* statement handle they prepare(). This also supports use of native octet encodings of non-UTF-8 filenames. The existing connect functions are re-implemented in terms of connectSqlite3Ext. When auto-finish is off, it makes no sense to track the open statement handles that only the user will close. Therefore, ChildList becomes a (Maybe ChildList) and is unused when auto-finish is disabled. Since we're not finishing queries when the last result row is read, we at least promptly "reset" the statement, freeing up some of the underlying resources. Finally, single-use statements that the library prepares for its own internal purposes are never auto-finished, and finished explicitly instead. Most of these will become cached statements in the next commit. The previous implementation of clone() was incorrect, it forgot the filename encoding, this is now preserved along with the auto-finish flag.
We don't need a statement handle for these, just call sqlite3_exec() via fexecuteRaw(). Also avoid creating a persistent statement handle for the gettables query and the one-shot queries processed by run and runRaw.
We'll use the "live" values with requests for an already prepared statement, and only fall-back to the saved values when finished after a previous prepare in auto-finish mode.
With implicit auto-finish of course the finish happens at the right time automatically, but with auto-finish off, a bit more care is required to not reset the query before all the desired rows are examined. The explicit "finish" for the statements used to fetch all rows when using the lazy fetchAllRows needs to be delayed until the rows have been fully processed (by comparing with the expected values). This is not needed in the non-lazy fetchAllRows' case. Replaced quickQuery with safeQuickQuery' when not explicitly testing quickQuery. The latter is not safe when auto-finish is off. Applications that disable auto-finish must avoid functions like quickQuery and quickQuery' internally leak prepared statements. NB: quickQuery' could be changed to explicitly finish its statement handle.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Disabling auto-finish retains open prepared statement handles, which the application is responsible to manage and release before disconnect. For applications that frequently run the same query this can be a significant speedup. Some HDBC functions (queryQuery and quickQuery' are unsafe in this mode and should be avoided).
A mock-up of an an application that repeatedly queries for the same 8 rows 10000 times saw a change in runtime from 13.6 seconds to 5.7 seconds.