Optimize the checking logic for conversion to skiplist in advance.

[RayaCoo]

During the ZADD operation, a conversion from listpack to skiplist might be necessary for the sorted set. Currently, the function zsetTypeMaybeConvert only examines the number of elements but does not check the Max size of the elements. It is advisable to include a check for value_len_hint for a more robust conversion check mechanism.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.31%. Comparing base (36e81d9) to head (5964508).
Report is 14 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #806      +/-   ##
============================================
- Coverage     70.46%   70.31%   -0.15%     
============================================
  Files           112      112              
  Lines         61135    61315     +180     
============================================
+ Hits          43077    43115      +38     
- Misses        18058    18200     +142     

Files	Coverage Δ
src/t_zset.c	95.56% <100.00%> (+0.01%)	⬆️

... and 25 files with indirect coverage changes

[madolson]

@RayaCoo Can you fix the formatting errors?

I'm also curious if there is some specific use case this is trying to optimize. The size_hint is to optimize the case where the first addition is adding a lot of elements. I'm not sure how often there are cases where all items are that large. Is there a specific use case you had in mind?

[RayaCoo]

@RayaCoo Can you fix the formatting errors?

I'm also curious if there is some specific use case this is trying to optimize. The size_hint is to optimize the case where the first addition is adding a lot of elements. I'm not sure how often there are cases where all items are that large. Is there a specific use case you had in mind?

@madolson Format error has been fixed.
And here are some specific use cases and test results:

• First of all, this PR doesn't significantly affect the performance of ZADD.

unstable

./valkey-benchmark -P 10 -n 50000000 -t zadd
Summary:
  throughput summary: 474212.31 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
        0.972     0.264     0.927     1.271     1.319     3.847

this PR

./valkey-benchmark -P 10 -n 50000000 -t zadd
Summary:
  throughput summary: 473269.72 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
        0.975     0.208     0.935     1.279     1.327     3.095

• Secondly, when the ZADD command is used in the form ZADD key score_1 member_1 score_2 member_2 ... score_n member_n (where 2 <= n <= zset_max_listpack_entries), and the zset corresponding to the key does not exist. If we pass maxelelen to the zsetTypeCreate function instead of sdslen(c->argv[scoreidx+1]->ptr).
  In the specific case shows blow, the test result shows the performance can improve about 62%.

PS: Length of the last member is greater than default zset_max_listpack_value

unstable

./valkey-benchmark -P 10 -n 500000 eval "for i = 1, 100 do redis.call('zadd', KEYS[1], 1, 'e1', 2, 'e2',
3, 'e3', 4, 'e4', 5, 'e5', 6, 'e6', 7, 'e7', 8, 'e8', 9, 'e9', 10, 'e10', 11, 'e11', 12, 
'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); 
redis.call('del', 'mySortedSet'); end;" 1 mySortedSet
Summary:
  throughput summary: 747.14 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
      668.884    13.728   747.007   765.439   778.751   977.919

this PR

./valkey-benchmark -P 10 -n 500000 eval "for i = 1, 100 do redis.call('zadd', KEYS[1], 1, 'e1', 2, 'e2',
3, 'e3', 4, 'e4', 5, 'e5', 6, 'e6', 7, 'e7', 8, 'e8', 9, 'e9', 10, 'e10', 11, 'e11', 12,
'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); 
redis.call('del', 'mySortedSet'); end;" 1 mySortedSet
  
Summary:
  throughput summary: 1210.07 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
      412.941     8.504   461.311   473.855   482.815   601.599

• Third, when the ZADD command is used in the form ZADD key score_1 member_1 score_2 member_2 ... score_n member_n (where 2 <= n <= zset_max_listpack_entries), and the zset corresponding to the key exists., If we pass maxelelen to the zsetTypeMaybeConvert function as the value_len_hint parameter.
  In the specific case shows blow, the test result shows the performance can improve about 37%.

unstable

./valkey-benchmark -P 10 -n 500000 eval "for i = 1, 100 do redis.call('zadd', KEYS[1], 1, 'e1'); 
redis.call('zadd','mySortedSet', 2, 'e2', 3, 'e3', 4, 'e4', 5, 'e5', 6, 'e6', 7, 'e7', 8, 'e8', 9, 'e9', 
10, 'e10', 11, 'e11', 12, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); 
redis.call('del', 'mySortedSet'); end;" 1 mySortedSet

Summary:
  throughput summary: 664.18 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
      752.455    15.288   841.215   864.255   878.591  1099.775

this PR

./valkey-benchmark -P 10 -n 500000 eval "for i = 1, 100 do redis.call('zadd', KEYS[1], 1, 'e1'); 
redis.call('zadd','mySortedSet', 2, 'e2', 3, 'e3', 4, 'e4', 5, 'e5', 6, 'e6', 7, 'e7', 8, 'e8', 9, 'e9',
10, 'e10', 11, 'e11', 12, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); 
redis.call('del', 'mySortedSet'); end;" 1 mySortedSet

Summary:
  throughput summary: 907.95 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
      550.386    11.312   615.935   629.759   639.487   781.311

[soloestoy]

good job! @enjoy-binbin do you wanna take a look?

[enjoy-binbin]

LGTM.