Skip to content

Call makeObjectShared to normalize a shared object into a true shared object#1566

Merged
enjoy-binbin merged 12 commits into
valkey-io:unstablefrom
enjoy-binbin:shared_object
Jun 10, 2025
Merged

Call makeObjectShared to normalize a shared object into a true shared object#1566
enjoy-binbin merged 12 commits into
valkey-io:unstablefrom
enjoy-binbin:shared_object

Conversation

@enjoy-binbin

@enjoy-binbin enjoy-binbin commented Jan 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Currently, only int and shared.redacted call makeObjectShared for
shared objects.

Although this breaks the blame log, there is a few points i would
like to mention:

  1. For threading, our fork for some reasons calling decrRefCount in
    the threads for shared.command. Although this is not used in OSS
    code now, it may be a issue in the future.
  2. For cleanup, all shared objects's refcount should be the same.
  3. For CoW, probably can reduce CoW although it is very minor.

We decided in #2189 to keep extended-redis-compatibility for now.

@enjoy-binbin
Call makeObjectShared to normalize a shared object into a true shared…
37263f3 
… object

Currently, only int and shared.redacted call makeObjectShared for
shared objects.

Although this breaks the blame log, there is a few points i would
like to mention:
1. For threading, our fork for some reasons calling decrRefCount in
   the threads for shared.command. Although this is not used in OSS
   code now, it may be a issue in the future.
2. For cleanup, all shared objects's refcount should be the same.
3. For CoW, probably can reduce CoW although it is very minor.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@codecov

codecov Bot commented Jan 15, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.41%. Comparing base (0039adc) to head (7b9e7f5).
Report is 13 commits behind head on unstable.

Additional details and impacted files 
@@             Coverage Diff              @@
##           unstable    #1566      +/-   ##
============================================
- Coverage     71.45%   71.41%   -0.05%     
============================================
  Files           122      122              
  Lines         66210    66207       -3     
============================================
- Hits          47311    47280      -31     
- Misses        18899    18927      +28
Files with missing lines Coverage Δ
src/config.c 78.52% <100.00%> (ø)
src/server.c 87.93% <100.00%> (+<0.01%) ⬆️
src/server.h 100.00% <ø> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

enjoy-binbin
enjoy-binbin commented Jan 15, 2025
View reviewed changes
Comment thread src/server.c
@enjoy-binbin
handle extended_redis_compat
5c2fda5 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@zuiderkwast zuiderkwast requested a review from madolson January 15, 2025 09:17
@enjoy-binbin
Try to make the CI happy
aab12c0 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin
Merge remote-tracking branch 'upstream/unstable' into shared_object
6937994 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin
Revert "Try to make the CI happy"
24281ba 
This reverts commit aab12c0.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin
Revert "handle extended_redis_compat"
ec4f127 
This reverts commit 5c2fda5.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin
Revert "Call makeObjectShared to normalize a shared object into a tru…
1d4044b 
…e shared object"

This reverts commit 37263f3.

Signed-off-by: Binbin <binloveplay1314@qq.com>
enjoy-binbin
enjoy-binbin commented Jun 5, 2025
View reviewed changes
Comment thread src/server.c Outdated
zuiderkwast
zuiderkwast reviewed Jun 5, 2025
View reviewed changes
Comment thread src/server.c Outdated
Comment thread src/server.c Outdated
@enjoy-binbin
Try a new way to reduce the diff (force push for DCO)
58a69be 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin
Let see if this will be better
782e64a 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin enjoy-binbin added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Jun 5, 2025
zuiderkwast
zuiderkwast reviewed Jun 5, 2025
View reviewed changes
Comment thread src/server.c Outdated
zuiderkwast
zuiderkwast reviewed Jun 5, 2025
View reviewed changes
Comment thread src/server.c Outdated
@enjoy-binbin @zuiderkwast
Apply review suggestions
99a5062 
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast
zuiderkwast reviewed Jun 6, 2025
View reviewed changes
Comment thread src/server.c Outdated
@enjoy-binbin
Apply review suggestions
b2750ff 
Signed-off-by: Binbin <binloveplay1314@qq.com>
zuiderkwast
zuiderkwast approved these changes Jun 7, 2025
View reviewed changes
Comment thread src/server.c Outdated
@enjoy-binbin
update comments
7b9e7f5 
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin enjoy-binbin merged commit 1b531e5 into valkey-io:unstable Jun 10, 2025
60 checks passed
@enjoy-binbin enjoy-binbin deleted the shared_object branch June 10, 2025 11:48
chzhoo pushed a commit to chzhoo/valkey that referenced this pull request Jun 12, 2025
@enjoy-binbin @zuiderkwast @chzhoo
Call makeObjectShared to normalize a shared object into a true shared…
341bcad 
… object (valkey-io#1566)

Currently, only int and shared.redacted call makeObjectShared for
shared objects.

Although this breaks the blame log, there is a few points i would
like to mention:
1. For threading, our fork for some reasons calling decrRefCount in
   the threads for shared.command. Although this is not used in OSS
   code now, it may be a issue in the future.
2. For cleanup, all shared objects's refcount should be the same.
3. For CoW, probably can reduce CoW although it is very minor.

We decided in valkey-io#2189 to keep `extended-redis-compatibility` for now.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: chzhoo <czawyx@163.com>
shanwan1 pushed a commit to shanwan1/valkey that referenced this pull request Jun 13, 2025
@enjoy-binbin @zuiderkwast
Call makeObjectShared to normalize a shared object into a true shared…
dcdaa75 
… object (valkey-io#1566)

Currently, only int and shared.redacted call makeObjectShared for
shared objects.

Although this breaks the blame log, there is a few points i would
like to mention:
1. For threading, our fork for some reasons calling decrRefCount in
   the threads for shared.command. Although this is not used in OSS
   code now, it may be a issue in the future.
2. For cleanup, all shared objects's refcount should be the same.
3. For CoW, probably can reduce CoW although it is very minor.

We decided in valkey-io#2189 to keep `extended-redis-compatibility` for now.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: shanwan1 <shanwan1@intel.com>
@sundb sundb mentioned this pull request Oct 31, 2025
Open
ranshid added a commit to ranshid/valkey that referenced this pull request Mar 18, 2026
@ranshid
remove duplicated lline
7f4b67e 
Probably added by mistake during some merge of valkey-io#1566

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
@ranshid ranshid mentioned this pull request Mar 18, 2026
Merged
ranshid added a commit that referenced this pull request Mar 18, 2026
@ranshid
remove duplicated lline (#3379)
afe6ee1 
Probably added by mistake during some merge of #1566

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
JimB123 pushed a commit that referenced this pull request Mar 19, 2026
@ranshid @JimB123
remove duplicated lline (#3379)
5d283a2 
Probably added by mistake during some merge of #1566

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
roshkhatri pushed a commit to roshkhatri/valkey that referenced this pull request Mar 20, 2026
@ranshid @roshkhatri
remove duplicated lline (valkey-io#3379)
f24a9e6 
Probably added by mistake during some merge of valkey-io#1566

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 16, 2026
@ranshid @sarthakaggarwal97
remove duplicated lline (valkey-io#3379)
afb3b8b 
Probably added by mistake during some merge of valkey-io#1566

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
madolson pushed a commit that referenced this pull request Apr 27, 2026
@ranshid @madolson
remove duplicated lline (#3379)
9670256 
Probably added by mistake during some merge of #1566

Signed-off-by: Ran Shidlansik <ranshid@amazon.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
@madolson madolson Awaiting requested review from madolson

Assignees

No one assigned

Labels

run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)

Projects

Valkey 9.0
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@enjoy-binbin @zuiderkwast