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

proxy: ustats internal refactor #1149

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Conversation

dormando
Copy link
Member

If a ustat changes name for the same index, its value is reset to 0.

Cleans up some confusing code around how ustats are handled; the global ctx and worker threads now use different structures with different intent, instead of different sides of the same struct.

Also reorders the entries in the global ctx so the most frequently accessed ones are clustered toward the front of the struct.

@dormando dormando added the proxy worklogs and issues related to proxy label Jun 19, 2024
@dormando
Copy link
Member Author

this change prepares for a potential performance fix (at least benchmarking the effect of) of inlining the ustats names in the global struct vs using a pointer for each. If the stats proxy function has to stop and fetch lots of random areas of memory it can have significant performance drop.

However this needs to be proved and the code prepared for it since that change would use a little excess memory.

@dormando
Copy link
Member Author

eh. maybe it could "compile" the names down into a compact buffer during the reload stage. now that I have a separate structure for the config side vs the worker side that's pretty easy actually.

1024 stats would be over 130k of just name memory, which sucks if the names are much shorter than the limit (128 bytes).

debating just doing this now... if I also unroll the snprintf it'll get a significant CPU boost. then people using billions of counters wouldn't be punching themselves in the face as hard.

@dormando
Copy link
Member Author

This is done now. I'll let it sit for a while then review it and merge to staging.

Can see the difference in performance counters but it's pretty hard to test the impact of the performance fixes... If I ever care enough to quantify it I could see about raising the default stat count limit.

proto_proxy.c Outdated Show resolved Hide resolved
proxy.h Outdated Show resolved Hide resolved
If a ustat changes name for the same index, its value is reset to 0.

Cleans up some confusing code around how ustats are handled; the global
ctx and worker threads now use different structures with different
intent, instead of different sides of the same struct.

Speeds up `stats proxy` dumping by compacting stat names into a linear
buffer, and unrolls some snprintf's into direct memory copies plus a
faster itoa.

Also reorders the entries in the global ctx so the most frequently
accessed ones are clustered toward the front of the struct.
@dormando dormando merged commit ddc7398 into memcached:staging Jun 21, 2024
1 check passed
@dormando dormando added the staging complete but needs more testing before final merge label Jun 21, 2024
@dormando
Copy link
Member Author

merged to staging. fixes a lot of longstanding annoyances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proxy worklogs and issues related to proxy staging complete but needs more testing before final merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant