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.

Sign up for GitHub

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

Jump to bottom

Fix LUA <-> RESP2 Mappings, NoScript enforcement #946

Merged
merged 12 commits into from
Jan 24, 2025
Merged

Fix LUA <-> RESP2 Mappings, NoScript enforcement #946

merged 12 commits into from
Jan 24, 2025

Conversation

kevin-montrose
Copy link
Contributor

@kevin-montrose kevin-montrose commented Jan 22, 2025 •
edited
Loading

Our command lookup in RespServerSession is getting complicated, and now pretty conditional (are ACLs on?, are we in a script?, array command?, admin command?, cluster slots checks?, etc.)...

I'm tempted to draft up a proposal for a refactor there... but not as part of this.

TODO:

  • Needs benchmarking, this affects the core command dispatch loop
  • Full test pass

Benchmarks

Since adds a new check in the command dispatch path, looking at some non-Lua benchmarks.

main is at 51ee0f0b9f2411854b0ba1410d0b130168af3825
issue939 is at bea15117502debf90391a4afab24e9c18a636976

RawStringOperations

No big swings, just some jitter looks like.

main

Method Params Mean Error StdDev Allocated
Set ACL 13.734 us 0.0710 us 0.0629 us -
SetEx ACL 20.135 us 0.1142 us 0.1013 us -
SetNx ACL 20.937 us 0.1094 us 0.1023 us -
SetXx ACL 21.675 us 0.0892 us 0.0697 us -
GetFound ACL 13.683 us 0.1301 us 0.1217 us -
GetNotFound ACL 9.595 us 0.0426 us 0.0378 us -
Increment ACL 22.214 us 0.1517 us 0.1419 us -
Decrement ACL 21.793 us 0.1974 us 0.1750 us -
IncrementBy ACL 25.112 us 0.2949 us 0.2614 us -
DecrementBy ACL 25.928 us 0.1231 us 0.1152 us -
Set AOF 19.986 us 0.1732 us 0.1620 us -
SetEx AOF 23.810 us 0.1948 us 0.1727 us -
SetNx AOF 26.919 us 0.5369 us 0.5022 us -
SetXx AOF 28.692 us 0.2709 us 0.2262 us -
GetFound AOF 13.191 us 0.0477 us 0.0423 us -
GetNotFound AOF 9.356 us 0.0568 us 0.0532 us -
Increment AOF 27.870 us 0.1722 us 0.1527 us -
Decrement AOF 33.978 us 0.2803 us 0.2622 us -
IncrementBy AOF 30.094 us 0.1805 us 0.1507 us -
DecrementBy AOF 29.881 us 0.2338 us 0.2187 us -
Set None 13.854 us 0.0829 us 0.0776 us -
SetEx None 18.391 us 0.1312 us 0.1163 us -
SetNx None 20.912 us 0.1831 us 0.1713 us -
SetXx None 21.864 us 0.0679 us 0.0602 us -
GetFound None 13.812 us 0.1151 us 0.1077 us -
GetNotFound None 9.547 us 0.0386 us 0.0361 us -
Increment None 21.076 us 0.0977 us 0.0914 us -
Decrement None 21.433 us 0.1223 us 0.1084 us -
IncrementBy None 24.678 us 0.1319 us 0.1169 us -
DecrementBy None 25.567 us 0.2372 us 0.2218 us -

issue939

Method Params Mean Error StdDev Allocated
Set ACL 14.137 us 0.0851 us 0.0754 us -
SetEx ACL 19.015 us 0.1794 us 0.1678 us -
SetNx ACL 21.305 us 0.1600 us 0.1497 us -
SetXx ACL 22.682 us 0.1106 us 0.0981 us -
GetFound ACL 13.860 us 0.0689 us 0.0611 us -
GetNotFound ACL 9.727 us 0.0600 us 0.0561 us -
Increment ACL 21.124 us 0.1535 us 0.1361 us -
Decrement ACL 21.440 us 0.1018 us 0.0902 us -
IncrementBy ACL 25.350 us 0.1357 us 0.1269 us -
DecrementBy ACL 24.912 us 0.1111 us 0.0985 us -
Set AOF 20.070 us 0.2995 us 0.2802 us -
SetEx AOF 24.171 us 0.2351 us 0.2084 us -
SetNx AOF 26.698 us 0.2560 us 0.2395 us -
SetXx AOF 28.315 us 0.2004 us 0.1776 us -
GetFound AOF 13.779 us 0.0331 us 0.0276 us -
GetNotFound AOF 9.502 us 0.0625 us 0.0554 us -
Increment AOF 25.650 us 0.1028 us 0.0911 us -
Decrement AOF 26.222 us 0.1693 us 0.1500 us -
IncrementBy AOF 35.864 us 0.5301 us 0.4958 us -
DecrementBy AOF 29.753 us 0.2283 us 0.2024 us -
Set None 13.741 us 0.0419 us 0.0327 us -
SetEx None 18.564 us 0.1278 us 0.1133 us -
SetNx None 20.607 us 0.0740 us 0.0618 us -
SetXx None 22.152 us 0.1097 us 0.0972 us -
GetFound None 13.503 us 0.1122 us 0.1049 us -
GetNotFound None 10.168 us 0.0569 us 0.0505 us -
Increment None 21.129 us 0.0996 us 0.0932 us -
Decrement None 22.040 us 0.0735 us 0.0651 us -
IncrementBy None 25.513 us 0.1237 us 0.1097 us -
DecrementBy None 25.268 us 0.1444 us 0.1350 us -

ScriptOperations

Similary, some jitter, but no big swings in either direction.

main

Method Params Mean Error StdDev Gen0 Gen1 Gen2 Allocated
ScriptLoad Managed,Limit 85.44 us 0.915 us 0.811 us - - - 9600 B
ScriptExistsTrue Managed,Limit 19.66 us 0.136 us 0.120 us - - - -
ScriptExistsFalse Managed,Limit 18.47 us 0.065 us 0.054 us - - - -
Eval Managed,Limit 60.99 us 0.403 us 0.357 us - - - -
EvalSha Managed,Limit 26.84 us 0.171 us 0.151 us - - - -
SmallScript Managed,Limit 52.09 us 0.383 us 0.340 us - - - -
LargeScript Managed,Limit 4,748.40 us 38.851 us 32.442 us - - - -
ArrayReturn Managed,Limit 150.64 us 9.967 us 29.389 us - - - -
ScriptLoad Managed,None 83.53 us 0.229 us 0.214 us - - - 9600 B
ScriptExistsTrue Managed,None 19.78 us 0.188 us 0.176 us - - - -
ScriptExistsFalse Managed,None 18.41 us 0.087 us 0.081 us - - - -
Eval Managed,None 61.29 us 0.395 us 0.370 us - - - -
EvalSha Managed,None 26.97 us 0.218 us 0.204 us - - - -
SmallScript Managed,None 52.21 us 0.171 us 0.152 us - - - -
LargeScript Managed,None 4,827.43 us 63.552 us 59.447 us - - - 4 B
ArrayReturn Managed,None 153.15 us 12.371 us 36.477 us - - - -
ScriptLoad Native,None 84.11 us 0.407 us 0.381 us - - - 9600 B
ScriptExistsTrue Native,None 19.63 us 0.186 us 0.174 us - - - -
ScriptExistsFalse Native,None 18.68 us 0.089 us 0.083 us - - - -
Eval Native,None 60.84 us 0.416 us 0.369 us - - - -
EvalSha Native,None 26.55 us 0.074 us 0.066 us - - - -
SmallScript Native,None 52.70 us 0.208 us 0.184 us - - - -
LargeScript Native,None 4,255.21 us 67.183 us 62.843 us - - - 5 B
ArrayReturn Native,None 108.81 us 0.449 us 0.398 us - - - -
ScriptLoad Tracked,Limit 83.98 us 0.463 us 0.433 us - - - 9600 B
ScriptExistsTrue Tracked,Limit 20.38 us 0.203 us 0.190 us - - - -
ScriptExistsFalse Tracked,Limit 19.22 us 0.210 us 0.197 us - - - -
Eval Tracked,Limit 61.09 us 1.069 us 1.000 us - - - -
EvalSha Tracked,Limit 26.87 us 0.091 us 0.076 us - - - -
SmallScript Tracked,Limit 52.56 us 0.268 us 0.238 us - - - -
LargeScript Tracked,Limit 5,054.05 us 30.483 us 27.022 us 15.6250 15.6250 15.6250 25 B
ArrayReturn Tracked,Limit 131.51 us 1.249 us 1.107 us - - - -
ScriptLoad Tracked,None 84.05 us 0.524 us 0.490 us - - - 9600 B
ScriptExistsTrue Tracked,None 20.94 us 0.135 us 0.126 us - - - -
ScriptExistsFalse Tracked,None 18.94 us 0.109 us 0.097 us - - - -
Eval Tracked,None 61.35 us 0.297 us 0.248 us - - - -
EvalSha Tracked,None 31.24 us 0.218 us 0.193 us - - - -
SmallScript Tracked,None 53.48 us 0.279 us 0.248 us - - - -
LargeScript Tracked,None 4,996.38 us 39.261 us 36.725 us 15.6250 15.6250 15.6250 18 B
ArrayReturn Tracked,None 129.89 us 1.272 us 1.190 us - - - -

issue939

Method Params Mean Error StdDev Gen0 Gen1 Gen2 Allocated
ScriptLoad Managed,Limit 85.45 us 0.327 us 0.273 us - - - 9600 B
ScriptExistsTrue Managed,Limit 19.53 us 0.156 us 0.146 us - - - -
ScriptExistsFalse Managed,Limit 18.72 us 0.094 us 0.088 us - - - -
Eval Managed,Limit 60.28 us 0.296 us 0.247 us - - - -
EvalSha Managed,Limit 27.01 us 0.281 us 0.263 us - - - -
SmallScript Managed,Limit 59.57 us 0.261 us 0.231 us - - - -
LargeScript Managed,Limit 4,798.25 us 45.370 us 42.439 us - - - 3 B
ArrayReturn Managed,Limit 154.03 us 12.482 us 36.803 us - - - -
ScriptLoad Managed,None 84.08 us 0.522 us 0.489 us - - - 9600 B
ScriptExistsTrue Managed,None 19.56 us 0.077 us 0.072 us - - - -
ScriptExistsFalse Managed,None 19.08 us 0.130 us 0.122 us - - - -
Eval Managed,None 61.29 us 0.363 us 0.322 us - - - -
EvalSha Managed,None 26.73 us 0.186 us 0.174 us - - - -
SmallScript Managed,None 55.61 us 0.367 us 0.325 us - - - -
LargeScript Managed,None 4,856.97 us 46.176 us 43.193 us - - - 6 B
ArrayReturn Managed,None 156.27 us 12.554 us 37.017 us - - - -
ScriptLoad Native,None 83.75 us 0.417 us 0.390 us - - - 9600 B
ScriptExistsTrue Native,None 20.35 us 0.309 us 0.289 us - - - -
ScriptExistsFalse Native,None 18.65 us 0.142 us 0.118 us - - - -
Eval Native,None 61.54 us 0.406 us 0.360 us - - - -
EvalSha Native,None 28.14 us 0.341 us 0.302 us - - - -
SmallScript Native,None 53.48 us 0.414 us 0.346 us - - - -
LargeScript Native,None 4,190.35 us 21.821 us 18.222 us - - - 3 B
ArrayReturn Native,None 111.29 us 0.589 us 0.551 us - - - -
ScriptLoad Tracked,Limit 84.76 us 0.726 us 0.643 us - - - 9600 B
ScriptExistsTrue Tracked,Limit 20.08 us 0.110 us 0.097 us - - - -
ScriptExistsFalse Tracked,Limit 18.69 us 0.082 us 0.076 us - - - -
Eval Tracked,Limit 60.44 us 0.304 us 0.284 us - - - -
EvalSha Tracked,Limit 27.01 us 0.168 us 0.140 us - - - -
SmallScript Tracked,Limit 52.90 us 0.209 us 0.196 us - - - -
LargeScript Tracked,Limit 5,000.15 us 47.671 us 44.592 us 15.6250 15.6250 15.6250 21 B
ArrayReturn Tracked,Limit 130.19 us 1.165 us 1.090 us - - - -
ScriptLoad Tracked,None 84.43 us 0.383 us 0.340 us - - - 9600 B
ScriptExistsTrue Tracked,None 19.62 us 0.136 us 0.113 us - - - -
ScriptExistsFalse Tracked,None 18.71 us 0.083 us 0.070 us - - - -
Eval Tracked,None 61.16 us 0.331 us 0.310 us - - - -
EvalSha Tracked,None 26.49 us 0.109 us 0.097 us - - - -
SmallScript Tracked,None 53.33 us 0.229 us 0.214 us - - - -
LargeScript Tracked,None 5,086.21 us 49.271 us 46.088 us 15.6250 15.6250 15.6250 16 B
ArrayReturn Tracked,None 130.54 us 1.108 us 1.036 us - - - -

@kevin-montrose kevin-montrose marked this pull request as ready for review January 23, 2025 21:41
@badrishc badrishc merged commit 128bec3 into main Jan 24, 2025
18 checks passed
@badrishc badrishc deleted the issue939 branch January 24, 2025 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@badrishc badrishc badrishc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Index was outside the bounds of the array" error using Microsoft.Web.RedisSessionStateProvider
2 participants
@kevin-montrose @badrishc