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

Fix Latency Tracking Issue #659

Merged
merged 14 commits into from
Sep 16, 2024
Merged

Conversation

vazois
Copy link
Contributor

@vazois vazois commented Sep 13, 2024

This PR fixes a latency tracking issue that was caused by moving INFO under ProcessBasicCommands. Because of this, the latency tracking numbers for NET_RS event were not accurate.
We now implicitly consider any command under ProcessBasicCommands and ProcessArrayCommands as "fast" and all other commands (under ProcessOtherCommands and ProcessAdminCommands) as "slow".
For this reason, this PR also moves certain commands that are considered to be slow under ProcessOtherCommands.

Notes Explicit classification was dropped, and we use the implicit method. The below info is just for future reference. The alternative solution was to explicitly classify the command using the RespCommandsInfo (https://github.com//pull/659/commits/6e5cbc2f63374bd7c904f9a863d0926deff7c400) flags (i.e. "slow", "fast"). However, this solution incurs the following penalty as measured using RepsParseStress below.

Main

Method Mean Error StdDev Allocated
InlinePing 2.230 us 0.0033 us 0.0030 us -
Set 17.073 us 0.0521 us 0.0462 us -
SetEx 24.267 us 0.0478 us 0.0448 us -
Get 12.646 us 0.0258 us 0.0241 us -
ZAddRem 173.760 us 0.8113 us 0.7589 us 26624 B
LPushPop 142.637 us 1.8688 us 1.7481 us 30721 B
SAddRem 112.647 us 0.4345 us 0.4064 us 16384 B
HSetDel 155.358 us 1.4137 us 1.3224 us 55297 B
MyDictSetGet 244.953 us 1.2858 us 1.2027 us 30720 B

With checking IsDataCommand()

Method Mean Error StdDev Allocated
InlinePing 2.443 us 0.0036 us 0.0032 us -
Set 15.940 us 0.0123 us 0.0102 us -
SetEx 23.955 us 0.0552 us 0.0516 us -
Get 12.780 us 0.0167 us 0.0148 us -
ZAddRem 158.164 us 0.5077 us 0.4501 us 26624 B
LPushPop 152.503 us 1.5758 us 1.4740 us 30721 B
SAddRem 115.242 us 0.7184 us 0.6719 us 16384 B
HSetDel 164.699 us 2.0440 us 1.9119 us 55297 B
MyDictSetGet 238.705 us 0.5843 us 0.5465 us 30720 B

Using RespCommandsInfo

Method Mean Error StdDev Allocated
InlinePing 2.440 us 0.0008 us 0.0006 us -
Set 16.672 us 0.0274 us 0.0256 us -
SetEx 24.646 us 0.0559 us 0.0496 us -
Get 13.061 us 0.0126 us 0.0118 us -
ZAddRem 156.820 us 0.8423 us 0.7879 us 26624 B
LPushPop 147.941 us 1.0840 us 0.9610 us 30721 B
SAddRem 119.299 us 0.4573 us 0.4278 us 16384 B
HSetDel 164.228 us 1.3445 us 1.1919 us 55297 B
MyDictSetGet 236.392 us 1.7909 us 1.6752 us 30720 B

@vazois vazois marked this pull request as ready for review September 14, 2024 00:29
@vazois vazois merged commit f75773c into microsoft:main Sep 16, 2024
15 checks passed
@vazois vazois deleted the vazois/fix-latency-tracking branch September 17, 2024 18:05
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.

2 participants