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

Add FUNCTION LOAD and FCALL benchmark #1578

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from

Conversation

rjd15372
Copy link
Contributor

This commit adds two new benhchmarks to valkey-benchmark tool.

The FUNCTION LOAD benchmark evaluates the performance of loading new Lua scripts. This benchmark is parameterized by the number of functions that are present in the script. To control the number of functions we can use the CLI option --num-functions-in-script.

The FCALL benchmarks evaluates the perfomance of calling a Lua function of a previously loaded script. This benchmark is parameterized by the number of keys that are passed in the FCALL command. To control the number of keys we can use the CLI option --num-keys-in-fcall.

This commit adds two new benhchmarks to `valkey-benchmark` tool.

The `FUNCTION LOAD` benchmark evaluates the performance of loading new
Lua scripts. This benchmark is parameterized by the number of functions
that are present in the script. To control the number of functions we
can use the CLI option `--num-functions-in-script`.

The `FCALL` benchmarks evaluates the perfomance of calling a Lua
function of a previously loaded script. This benchmark is parameterized
by the number of keys that are passed in the `FCALL` command. To
control the number of keys we can use the CLI option
`--num-keys-in-fcall`.

Signed-off-by: Ricardo Dias <[email protected]>
@rjd15372 rjd15372 marked this pull request as ready for review January 17, 2025 10:21
@rjd15372 rjd15372 requested a review from zuiderkwast January 17, 2025 10:21
@rjd15372
Copy link
Contributor Author

I'll be adding more scripting engine related benchmarks to valkey-benchmark in follow up PRs

while (num_functions > 0) {
assert(buffer_len - written > 0);
if (with_keys) {
written += snprintf(buffer + written, buffer_len - written,

Check failure

Code scanning / CodeQL

Potentially overflowing call to snprintf High

The
size argument
of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.
"local function foo%u(keys, args)\nreturn keys[0]\nend\n",
num_functions);
} else {
written += snprintf(buffer + written, buffer_len - written,

Check failure

Code scanning / CodeQL

Potentially overflowing call to snprintf High

The
size argument
of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.
"local function foo%u()\nreturn 0\nend\n",
num_functions);
}
written += snprintf(buffer + written, buffer_len - written,

Check failure

Code scanning / CodeQL

Potentially overflowing call to snprintf High

The
size argument
of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.83%. Comparing base (921ba19) to head (ed83969).
Report is 4 commits behind head on unstable.

Files with missing lines Patch % Lines
src/valkey-benchmark.c 96.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1578      +/-   ##
============================================
- Coverage     70.98%   70.83%   -0.16%     
============================================
  Files           120      121       +1     
  Lines         65095    65182      +87     
============================================
- Hits          46210    46170      -40     
- Misses        18885    19012     +127     
Files with missing lines Coverage Δ
src/valkey-benchmark.c 61.64% <96.00%> (+1.50%) ⬆️

... and 16 files with indirect coverage changes

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.

1 participant