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 Reserve() to AutoProbing #37

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jelmervdl
Copy link
Contributor

Adds a Reserve() method to AutoProbing that is necessary to get Bitextor's document aligner to work with this type of table.

I tried multiple setups where I just initialised the main table with a different size, but making sure the destination table is large enough with a Reserve call seems to be the only way to have any predictably good performance.

I also added a move-assignment operator to scoped_memory, the lack thereof prevented me to do df = std::move(local_df).

Things that can be improved:

  • I didn't check if AutoProbing's auto-generated move assignment operator is up to the job, but the output seems to be as expected. The AutoProbing table that was moved out from might be in a funny state though, with its counters not reset and no backing memory left.
  • Reserve() is just a while-loop around Double(), which is the laziest implementation possible but does a couple of pointless realloc calls.

jelmervdl added 2 commits July 3, 2023 16:22
An implementation that's not re-allocating in a while loop is left as an exercise to the reader
@jelmervdl jelmervdl changed the title Prob hash table reserve Add Reserve() to AutoProbing Jul 3, 2023
@jelmervdl jelmervdl marked this pull request as draft July 3, 2023 15:53
@jelmervdl
Copy link
Contributor Author

Hmm, looking at this in jelmervdl/bitextor#3 and using this table doesn't really yield an improvement over std::unordered_map in terms of memory usage.

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.

1 participant