• yoav-steinberg's avatar
    acl check api for functions and eval (#10220) · 9dfeda58
    yoav-steinberg authored
    Changes:
    1. Adds the `redis.acl_check_cmd()` api to lua scripts. It can be used to check if the
      current user has permissions to execute a given command. The new function receives
      the command to check as an argument exactly like `redis.call()` receives the command
      to execute as an argument.
    2. In the PR I unified the code used to convert lua arguments to redis argv arguments from
      both the new `redis.acl_check_cmd()` API and the `redis.[p]call()` API. This cleans up
      potential duplicate code.
    3. While doing the refactoring in 2 I noticed there's an optimization to reduce allocation calls
      when parsing lua arguments into an `argv` array in the `redis.[p]call()` implementation.
      These optimizations were introduced years ago in 48c49c48
      and 4f686555. It is unclear why this was added.
      The original commit message claims a 4% performance increase which I couldn't recreate
      and might not be worth it even if it did recreate. This PR removes that optimization.
      Following are details of the benchmark I did that couldn't reveal any performance
      improvements due to this optimization:
    
    ```
    benchmark 1: src/redis-benchmark -P 500 -n 10000000 eval 'return redis.call("ping")' 0
    benchmark 2: src/redis-benchmark -P 500 -r 1000 -n 1000000 eval 'return redis.call("mset","k1__rand_int__","v1__rand_int__","k2__rand_int__","v2__rand_int__","k3__rand_int__","v3__rand_int__","k4__rand_int__","v4__rand_int__")' 0
    benchmark 3: src/redis-benchmark -P 500 -r 1000 -n 100000 eval "for i=1,100,1 do redis.call('set','kk'..i,'vv'..__rand_int__) end return redis.call('get','kk5')" 0
    benchmark 4: src/redis-benchmark -P 500 -r 1000 -n 1000000 eval 'return redis.call("mset","k1__rand_int__","v1__rand_int__","k2__rand_int__","v2__rand_int__","k3__rand_int__","v3__rand_int__","k4__rand_int__","v4__rand_int__xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")'
    ```
    I ran the benchmark on this branch with and without commit 68b71680a4d3bb8f0509e06578a9f15d05b92a47
    Results in requests per second:
    cmd | without optimization | without optimization 2nd run | with original optimization | with original optimization 2nd run
    -- | -- | -- | -- | --
    1 | 461233.34 | 477395.31 | 471098.16 | 469946.91
    2 | 34774.14 | 35469.8 | 35149.38 | 34464.93
    3 | 6390.59 | 6281.41 | 6146.28 | 6464.12
    4 | 28005.71 |   | 27965.77 |  
    
    As you can see, different use cases showed identical or negligible performance differences.
    So finally I decided to chuck the original optimization and simplify the code.
    9dfeda58
scripting.tcl 48.1 KB