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

Doc request: what modification is allowed to RawTable while holding RawIter #165

Closed
CAD97 opened this issue Jun 21, 2020 · 3 comments · Fixed by #184
Closed

Doc request: what modification is allowed to RawTable while holding RawIter #165

CAD97 opened this issue Jun 21, 2020 · 3 comments · Fixed by #184

Comments

@CAD97
Copy link
Contributor

CAD97 commented Jun 21, 2020

More context on urlo and my questionable code.

What operations are allowed on RawTable while holding a RawIter?

Per my reading, the implementation currently suggests:

  • Required(?) for exposed behavior:
    • non-mutating inspection of the RawTable
    • eraseing the most recently yielded Bucket
  • Definitely not allowed (UB):
    • anything that causes a reallocation
    • erasing a bucket "ahead" of the iterator that has yet to be yielded
  • Potentially allowed:
    • erasing a bucket "behind" the iterator head that has previously been yielded but is not the most recently yielded bucket

I would benefit from the above "delayed erasing" being guaranteed sound (for the use case linked above).

Side note: it would be useful to make RawIter's debug_assert that it iterated all the expected items say what went wrong. Something simple like "exhausted iterator before expected; this likely indicates modification of a hash table while iterating it, which is UB" would definitely make hitting that assertion more obvious than the default message of "200 != 0" from inside of hashbrown's internals.

@Amanieu
Copy link
Member

Amanieu commented Jun 21, 2020

This feels like something extremely fragile to rely on since we could, for example, in the future decide to iterate backwards because we've changed our internal representation.

Looking at your code, is it really necessary to process the to_drop list while iterating? Can't you just process that list after the initial iteration to find dead "roots" has finished?

@CAD97
Copy link
Contributor Author

CAD97 commented Jun 21, 2020

Yeah, after sleeping on it, I realize that doing that would work, and do so without any impact on asymptotic complexity or space usage, and negligible impact on actual space usage.

It'd still probably be nice to say what's guaranteed to work: at the least, erasing the bucket most recently yielded by the iterator needs to be allowed for retain to work IIUC.

@jonhoo
Copy link
Contributor

jonhoo commented Jul 1, 2020

@CAD97 I'm not sure if it's relevant to your use-case, but #167 may be of interest to you.

Amanieu added a commit to Amanieu/hashbrown that referenced this issue Aug 8, 2020
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 a pull request may close this issue.

3 participants