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

stats: when hot-restart is disabled, each stat still consumes maxNameLength() bytes for the name. #3508

Closed
jmarantz opened this issue May 30, 2018 · 2 comments
Assignees
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@jmarantz
Copy link
Contributor

Description:

When storing stats in shared memory for hot-restart, we preallocate max_stats * maxNameLength for string storage. But this means you have to make hard up-front decisions about how much memory you can spend. However if we disable hot-restart, there's no compelling reason to pad each stat, so you could just set maxNameLength pretty high. If that were fixed, we wouldn't have to tune maxNameLength so carefully.

@ambuc may wind up looking at this as he was the last one to dive into the not-hot-restart case.

@jmarantz
Copy link
Contributor Author

jmarantz commented May 31, 2018

More thoughts on this in light of the #3506 which forced me to look at the stats code again and realize this problem:

I think a solution to this problem that will improve the maintainability of the codebase is to remove the following methods & data from Stats::RawStatsData

  • configure(options)
  • configureForTestsOnly(options)
  • maxNameLength()
  • maxObjNameLength()
  • nameSize()
  • static initializeAndGetMutableMaxObjNameLength()
    and replace them all with:
  • static calculateRequiredSize(size_t name_length)

In the context of a hot-restart allocation, the calculateRequiredSize will be called with options_->maxObjNameLength(). In the case of a non-hot-restart allocation, it will be called with actual_name.size(). Then you can remove a whole bunch of hacks in the test dealing with the static underneath initializeAndGetMutableMaxObjNameLength.

@mattklein123
Copy link
Member

@jmarantz big +1 on killing the statics. I hate them also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

No branches or pull requests

4 participants