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.

Sign up for GitHub

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

Jump to bottom

Make use of a single listNode pointer for blocking utility lists #919

Merged
merged 5 commits into from
Aug 20, 2024
Merged

Make use of a single listNode pointer for blocking utility lists #919

merged 5 commits into from
Aug 20, 2024

Conversation

ranshid
Copy link
Member

@ranshid ranshid commented Aug 19, 2024 •
edited by enjoy-binbin
Loading

Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references.

This is to address This comment which was left as part of #787.

@ranshid
Make use of a single listNode pointer for blocking utility lists
9c7a4b6 
This is to address [This comment](valkey-io#787 (comment))
which was left as part of valkey-io#787.

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
@codecov Codecov
Copy link

codecov bot commented Aug 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.21%. Comparing base (7795152) to head (0bfdf50).
Report is 1 commits behind head on unstable.

Additional details and impacted files 
@@             Coverage Diff              @@
##           unstable     #919      +/-   ##
============================================
- Coverage     70.37%   70.21%   -0.17%     
============================================
  Files           112      112              
  Lines         61505    61506       +1     
============================================
- Hits          43286    43184     -102     
- Misses        18219    18322     +103
Files Coverage Δ
src/blocked.c 91.69% <100.00%> (+0.05%) ⬆️
src/networking.c 88.81% <ø> (-0.01%) ⬇️
src/server.h 100.00% <ø> (ø)

... and 10 files with indirect coverage changes

@ranshid
format fixes
27b1c51 
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
zuiderkwast
zuiderkwast reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Great!

src/server.h Outdated Show resolved Hide resolved
@ranshid @zuiderkwast
Update src/server.h
1f470f9 
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
enjoy-binbin
enjoy-binbin approved these changes Aug 19, 2024
View reviewed changes
Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

LGTM.

hpatro
hpatro approved these changes Aug 19, 2024
View reviewed changes
src/server.h Outdated Show resolved Hide resolved
src/server.h Show resolved Hide resolved
ranshid
ranshid commented Aug 20, 2024
View reviewed changes
src/server.h Outdated Show resolved Hide resolved
@ranshid
Update src/server.h
5fcc6d8 
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
enjoy-binbin
enjoy-binbin reviewed Aug 20, 2024
View reviewed changes
src/server.h Outdated Show resolved Hide resolved
@enjoy-binbin
Fix odd format warning
0bfdf50 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin enjoy-binbin merged commit e2ab7ff into valkey-io:unstable Aug 20, 2024
47 checks passed
mapleFU pushed a commit to mapleFU/valkey that referenced this pull request Aug 21, 2024
@ranshid @zuiderkwast
@enjoy-binbin @mapleFU
Make use of a single listNode pointer for blocking utility lists (val…
deed6ed 
…key-io#919)

Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references.

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: mwish <maplewish117@gmail.com>
mapleFU pushed a commit to mapleFU/valkey that referenced this pull request Aug 22, 2024
@ranshid @zuiderkwast
@enjoy-binbin @mapleFU
Make use of a single listNode pointer for blocking utility lists (val…
2f25c20 
…key-io#919)

Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references.

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: mwish <maplewish117@gmail.com>
madolson pushed a commit that referenced this pull request Sep 2, 2024
@ranshid @zuiderkwast
@enjoy-binbin @madolson
Make use of a single listNode pointer for blocking utility lists (#919)
14d3588 
Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references. 

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
madolson pushed a commit that referenced this pull request Sep 3, 2024
@ranshid @zuiderkwast
@enjoy-binbin @madolson
Make use of a single listNode pointer for blocking utility lists (#919)
57e6369 
Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references. 

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
@ranshid @zuiderkwast
@enjoy-binbin @PingXie
Make use of a single listNode pointer for blocking utility lists (val…
783418e 
…key-io#919)

Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references.

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
@ranshid @zuiderkwast
@enjoy-binbin @PingXie
Make use of a single listNode pointer for blocking utility lists (val…
ce08e2b 
…key-io#919)

Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references.

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Sep 14, 2024
@ranshid @zuiderkwast
@enjoy-binbin @PingXie
Make use of a single listNode pointer for blocking utility lists (val…
3fa4b60 
…key-io#919)

Saves some memory (one pointer) in the client struct.

Since a client cannot be blocked multiple times, we can assume
it will be held in only one extra utility list, so it is ok to maintain
a union of these listNode references.

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: ranshid <88133677+ranshid@users.noreply.github.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ping Xie <pingxie@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zuiderkwast zuiderkwast zuiderkwast approved these changes

@enjoy-binbin enjoy-binbin enjoy-binbin approved these changes

@hpatro hpatro hpatro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ranshid @zuiderkwast @enjoy-binbin @hpatro