Skip to content

Make Dict thread safe #1875

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

Merged
merged 5 commits into from
Apr 25, 2020
Merged

Make Dict thread safe #1875

merged 5 commits into from
Apr 25, 2020

Conversation

palaviv
Copy link
Contributor

@palaviv palaviv commented Apr 18, 2020

No description provided.

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a test build warning.


impl ThreadSafe for Dict {}

struct InnerDict<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit curious if PyDict and PySet can have RwLock<Dict<T>> instead of this InnerDict structure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, for separation of concerns Dict seems like it would work best as a standard-ish data structure that requires a mutable reference to mutate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a actually started that way but it became very complex. One of my goals is to not hold the lock while "python" code is running. In the Dict lookup we call __eq__ so we need to release that lock but in order to do that from PyDict and PySet we will need to split the implementation of Dict to smaller methods that do not call "python" code.
Do you guys think this is the way to go? That will make a lot of code duplication in PyDict and PySet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that is tricky. I don't really think I have a preference either way for mutable/interior mutable Dict.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will push this as other object are dependent on PyDictRef. We can always change the implementation in the future.

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.

3 participants