Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Replace unneeded seqcst with relaxed on atomic operations #4587

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

carllin
Copy link
Contributor

@carllin carllin commented Jun 7, 2019

Problem

Seqcst is used in unneeded areas

Summary of Changes

Replace seqcst with relaxed
Fixes #

@carllin carllin requested review from mvines and rob-solana June 7, 2019 00:00
@codecov
Copy link

codecov bot commented Jun 7, 2019

Codecov Report

Merging #4587 into master will increase coverage by 13.1%.
The diff coverage is 94.7%.

@@            Coverage Diff            @@
##           master   #4587      +/-   ##
=========================================
+ Coverage      66%   79.2%   +13.1%     
=========================================
  Files         181     181              
  Lines       38726   32293    -6433     
=========================================
+ Hits        25582   25584       +2     
+ Misses      13144    6709    -6435

1 similar comment
@codecov
Copy link

codecov bot commented Jun 7, 2019

Codecov Report

Merging #4587 into master will increase coverage by 13.1%.
The diff coverage is 94.7%.

@@            Coverage Diff            @@
##           master   #4587      +/-   ##
=========================================
+ Coverage      66%   79.2%   +13.1%     
=========================================
  Files         181     181              
  Lines       38726   32293    -6433     
=========================================
+ Hits        25582   25584       +2     
+ Misses      13144    6709    -6435

Copy link
Contributor

@rob-solana rob-solana left a comment

Choose a reason for hiding this comment

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

lgtm, but... why?

@carllin carllin merged commit c9d6320 into solana-labs:master Jun 7, 2019
@carllin
Copy link
Contributor Author

carllin commented Jun 7, 2019

@rob-solana, seeing seqcst implies an extra level of caution is needed when working with the code. There's also a small performance implication.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants