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

Restructuring, getting rid of more memory leaks, and improving readability #70

Open
wants to merge 66 commits into
base: main
Choose a base branch
from

Conversation

polterguy
Copy link
Contributor

Much better structure. Among some of the things I have done are;

  • Created SqlStatement class encapsulating all SQL statements executed towards SQLite
  • Moved things into relevant "xyz.h" file to separate concerns, making the code more readable
  • Applied better encapsulation and cohesion, in particular by expanding upon the vss_index class, moving parts it should be responsible for into it as methods
  • Data hiding by making the original structs into "real classes" with private and public members and methods

More work needs to be done, especially on the "xyz.h" files, that really should have associated "xyz.cpp" files, but at least it's a start. I tried to implement memory cache on the faiss indexes, and I had everything working, but didn't understand why the cache mechanism didn't work, until I realised SQLite will dynamically load the library on every connection, resulting in that my static std::map would be deleted, and hence my approach to caching was useless. I removed these parts, but you can see the idea in the history if interested.

Psst, you'll have to check out the branch in its entirety. There's no way you can make sense of the pull request by looking at the diff.

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