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
osd: avoid the config's get_val() overhead on the read path. #20217
Conversation
src/osd/PrimaryLogPG.cc
Outdated
auto osd_max_object_size = cct->_conf->get_val<uint64_t>( | ||
"osd_max_object_size"); | ||
cct->_conf->osd_skip_data_digest; | ||
const uint64_t osd_max_object_size = \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why newline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've though we would cross the 80th column boundary otherwise. Fortunately, just mislooked. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good except for that stray newline :)
Profiling shows the overhead of the md_config_t::get_val<T> can be significant. Unfortunately, it is being used in two critical places across the read path. The patch resorts to the legacy config infrastructure to mitigate the performance penalty. It is expected it will be superseded with a solution that allows to use the new, intended routines without hurting performance. ` Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
85eb4c0
to
64c7f31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will need to cache these settings, add add them to get_tracked_conf_keys()
Profiling shows the overhead of the
md_config_t::get_val<T>
can be significant. Unfortunately, it is being used in two critical places across the read path.The patch resorts to the legacy config infrastructure to mitigate the performance penalty. It is expected it will be superseded with a solution that allows to use the new, intended routines without hurting performance.
Before:
After:
Tested on the top of PR #19380 which means old
master
. However, the benefit could be higher now because of the recently introduced third call:cct->_conf->get_val<uint64_t>("osd_max_object_size")
.See also:
Signed-off-by: Radoslaw Zarzynski rzarzyns@redhat.com