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

pickerwrapper: use atomic instead of locks #7214

Merged
merged 2 commits into from
May 16, 2024
Merged

pickerwrapper: use atomic instead of locks #7214

merged 2 commits into from
May 16, 2024

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented May 8, 2024

RELEASE NOTES: none

Copy link
Contributor

@easwars easwars left a comment

Choose a reason for hiding this comment

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

Looks great!!

blockingCh chan struct{}
picker balancer.Picker
// If pickerGen holds a nil pointer, the pickerWrapper is closed.
pickerGen atomic.Pointer[pickerGeneration]
Copy link
Contributor

Choose a reason for hiding this comment

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

Wohooo, generics!!

Copy link
Member Author

Choose a reason for hiding this comment

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

I like atomic.Pointer a lot. atomic's unsafe.Pointer operations were terrible.

pw.pickerGen.Store(&pickerGeneration{
blockingCh: make(chan struct{}),
})
return pw
}

// updatePicker is called by UpdateBalancerState. It unblocks all blocked pick.
Copy link
Contributor

Choose a reason for hiding this comment

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

While you are here, could you please update this comment to say UpdateState instead of UpdateBalancerState. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Clarified that it comes from the LB policy since we have 16 different UpdateStates.

@easwars easwars assigned dfawley and unassigned easwars May 15, 2024
@dfawley dfawley added the Type: Performance Performance improvements (CPU, network, memory, etc) label May 16, 2024
@dfawley dfawley removed their assignment May 16, 2024
@dfawley dfawley merged commit e22436a into grpc:master May 16, 2024
12 checks passed
@dfawley dfawley deleted the pw branch May 16, 2024 20:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Performance Performance improvements (CPU, network, memory, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants