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

feat: merge waits and recvs to wrCounter #809

Merged
merged 9 commits into from
Mar 24, 2025

Conversation

SoulPancake
Copy link
Contributor

@rueian Please take a look at this
solves #807

@SoulPancake
Copy link
Contributor Author

taking a look at the test failure, race

@rueian
Copy link
Collaborator

rueian commented Mar 20, 2025

It looks like you didn't cast some of the return values correctly. I think we better have helper functions instead of scattering uint32( .... & 0xFFFFFFFF) and uint32( ... >>32) around the code base.

Maybe something like func IncrWaits() uint32, func DecrWaits() uint32, func DecrWaitsAndRecvs() (uint32, uint32), func LoadRecvs() uint32, and func LoadWaits() uint32

@SoulPancake
Copy link
Contributor Author

I did that now, still miss what went wrong, can you help me out? @rueian

@rueian
Copy link
Collaborator

rueian commented Mar 20, 2025

I did that now, still miss what went wrong, can you help me out? @rueian

^uint64(1) for decrement doesn't seem correct. Also, we need to merge p.DecrWaits() and p.IncrRecvs() into one p.DecrWaitsAndRecvs. That's the whole point of this improvement. Another thing is, my bad, we should make these helper functions private.

@rueian
Copy link
Collaborator

rueian commented Mar 20, 2025

So we have two cases for decrements

  1. p.decrWaits only decrements the lower 32bit. The upper 32bits should not be touched.
  2. p.decrWaitsAndRecvs decrements both upper 32bit and lower 32bit in one operation.

@SoulPancake
Copy link
Contributor Author

please take a look at it once @rueian

@SoulPancake
Copy link
Contributor Author

Please re-review @rueian Sorry for the delay

@rueian rueian merged commit 80509aa into redis:main Mar 24, 2025
34 checks passed
@rueian
Copy link
Collaborator

rueian commented Mar 24, 2025

Thanks so much @SoulPancake

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.

2 participants