Bug #51725
make bufferlist::c_str() skip rebuild when it isn't necessary
100%
Description
The ceph_msg_header2 on the other hand, the bufferlist comes from the segment data, which is also contiguous, but splicing out the segment crc causes the bufferlist to be composed of two buffers, the other one being an empty carriage.
So ceph_msg_header2 always have to be rebuilt, while the preamble and the epilogue do not.
I wonder if either `splice()` or `rebuild()` should be made smarter. From a `bufferlist` user POV, a splice from the end of the contiguous buffer leading to `c_str()` ending up in `rebuild()` and actually reallocating is counter-intuitive.
Good point.
I definitely think that `c_str()` can be made a little smarter to detect more cases that do not require rebuild.
Given that, perhaps it's a better approach to always prefer `reinterpret_cast`, and use `ceph::decode` only as a fallback.
In that case, this commit [https://github.com/ceph/ceph/pull/42280] will be reverted.
Related issues
History
#1 Updated by Radoslaw Zarzynski over 2 years ago
- Status changed from New to In Progress
#2 Updated by Kefu Chai over 2 years ago
- Status changed from In Progress to Fix Under Review
- Assignee set to Radoslaw Zarzynski
- Pull request ID set to 42417
#3 Updated by Ilya Dryomov over 2 years ago
- Status changed from Fix Under Review to Pending Backport
- Backport set to octopus,pacific
This spares a heap allocation on every received message. Marking for backporting to both octopus and pacific as a low-risk performance improvement.
#4 Updated by Backport Bot over 2 years ago
- Copied to Backport #52595: pacific: make bufferlist::c_str() skip rebuild when it isn't necessary added
#5 Updated by Backport Bot over 2 years ago
- Copied to Backport #52596: octopus: make bufferlist::c_str() skip rebuild when it isn't necessary added
#6 Updated by Backport Bot over 1 year ago
- Tags set to backport_processed
#7 Updated by Konstantin Shalygin about 24 hours ago
- Tracker changed from Feature to Bug
- Status changed from Pending Backport to Resolved
- % Done changed from 0 to 100
- Source set to Development
- Regression set to No
- Severity set to 3 - minor