Add length check before content comparison in equalStringObjects

[vitahlin]

Issue

Previously, even when only string equality needed to be determined, the comparison logic still performed unnecessary memcmp() calls to check string ordering, even if the lengths were not equal.

Change

~~This PR introduces a new comparison flag, STRING_COMPARE_EQUAL, to the compareStringObjectsWithFlags function. It aims to achieve efficient string equality checks while avoiding performing additional comparison operations.~~

This PR add length check before content comparison in equalStringObjects function.

Test result

I found that this function is called in watchCommand, so I verified the performance improvement of this PR using the WATCH command:

$ memtier_benchmark \
  --command="WATCH a aa aaa aaaa aaaaa b bb bbb bbbb bbbbb" \
  --test-time 60 \
  -c 1 \
  -t 1 \
. --hide-histogram

Before:

ALL STATS

==================================================================================================
Type         Ops/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
--------------------------------------------------------------------------------------------------
Watchs      43053.91         0.02273         0.02300         0.03900         0.06300      4666.98 
Totals      43053.91         0.02273         0.02300         0.03900         0.06300      9333.95 

After:

ALL STATS
==================================================================================================
Type         Ops/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
--------------------------------------------------------------------------------------------------
Watchs      43414.55         0.02216         0.02300         0.03900         0.05500      4706.07 
Totals      43414.55         0.02216         0.02300         0.03900         0.05500      9412.14

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.23%. Comparing base (daea05b) to head (eb11449). Report is 4 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2107      +/-   ##
============================================
- Coverage     71.25%   71.23%   -0.03%     
============================================
  Files           122      122              
  Lines         66026    66033       +7     
============================================
- Hits          47050    47037      -13     
- Misses        18976    18996      +20     

Files with missing lines	Coverage Δ
src/object.c	81.41% <100.00%> (+0.05%)	:arrow_up:

... and 11 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vitahlin]

Test result of latest commit:

Unstable:

ALL STATS
==================================================================================================
Type         Ops/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
--------------------------------------------------------------------------------------------------
Watchs      30603.56         0.03250         0.03100         0.06300         0.08700      3317.38 
Totals      30603.56         0.03250         0.03100         0.06300         0.08700      6634.76 

PR:

ALL STATS
==================================================================================================
Type         Ops/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
--------------------------------------------------------------------------------------------------
Watchs      30813.01         0.03227         0.03100         0.06300         0.08700      3340.08 
Totals      30813.01         0.03227         0.03100         0.06300         0.08700      6680.16 

[Fusl]

Shouldn't this length check be done inside compareStringObjectsWithFlags where we already grab alen and blen?

[zuiderkwast]

@Fusl see earlier comments

[Fusl]

@zuiderkwast Ah I missed the earlier comments since they were tucked away as resolved, that makes more sense now, thanks.