Skip to content

make dict iterator use atomic operation#2927

Merged
youknowone merged 2 commits into
RustPython:masterfrom
eldpswp99:make-dict-iterator-use-atomic-operation
Aug 22, 2021
Merged

make dict iterator use atomic operation#2927
youknowone merged 2 commits into
RustPython:masterfrom
eldpswp99:make-dict-iterator-use-atomic-operation

Conversation

@eldpswp99

Copy link
Copy Markdown
Contributor

in #2915, there was a comment to use only fetch_add, not using load & store to maintain atomic operation.

since dict iterator has this load & store, change to use fetch_add.

@youknowone youknowone requested review from DimitrisJim, coolreader18 and youknowone and removed request for youknowone August 22, 2021 06:19

@coolreader18 coolreader18 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! This is definitely better.

Comment thread vm/src/dictdatatype.rs Outdated
}
}

pub fn next_entry(&self, position: &AtomicCell<usize>) -> Option<(PyObjectRef, T)> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe this could be named next_entry_atomic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it better to revert _next_entry to next_entry when I change the function name to next_entry_atomic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@coolreader18 I sent new code with changing function name!

Comment thread vm/src/builtins/dict.rs
Comment thread vm/src/builtins/dict.rs
Comment thread vm/src/dictdatatype.rs
Comment thread vm/src/dictdatatype.rs
@youknowone youknowone force-pushed the make-dict-iterator-use-atomic-operation branch from a75c279 to 19ce637 Compare August 22, 2021 11:26
@youknowone youknowone merged commit 488e756 into RustPython:master Aug 22, 2021
@youknowone youknowone added the z-ca-2021 Tag to track contrubution-academy 2021 label Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2021 Tag to track contrubution-academy 2021

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants