----- Original Message ----- > On Thu, 3 Dec 2015, Casey Bodley wrote: > > > Well, yeah we are, it's just the actual Transaction structure which > > > wouldn't be dynamic -- the buffers and many other fields would still > > > hit the allocator. > > > -Sam > > > > Sure. I was looking specifically at the tradeoffs between allocating > > and moving the Transaction object itself. > > > > As it currently stands, the caller of ObjectStore can choose whether to > > allocate its Transactions on the heap, embed them in other objects, or > > put them on the stack for use with apply_transactions(). Switching to an > > interface built around unique_ptr forces all callers to use the heap. I'm > > advocating for an interface that doesn't. > > That leaves us with either std::move or.. the raw Transaction* we have > now. Right?
Right. The thing I really like about the unique_ptr approach is that the caller no longer has to care about the Transaction's lifetime, so doesn't have to allocate the extra ObjectStore::C_DeleteTransaction for cleanup. Movable Transactions accomplish this as well. Casey > > > > > It's true that the move ctor has to do work. I counted 18 fields, half > > > > of > > > > which are integers, and the rest have move ctors themselves. But the > > > > cpu > > > > is good at integers. The win here is that you're not hitting the > > > > allocator > > > > in the fast path. > > To be fair, many of these are also legacy that we can remove... possibly > even now. IIRC the only exposure to legacy encoded transactions (that use > the tbl hackery) are journal items from an upgrade pre-hammer OSD that > aren't flushed on upgrade. We should have made the osd flush the journal > before recording the 0_94_4 ondisk feature. We could add another one to > enforce that and rip all that code out now instead of waiting until > after jewel... that would be satisfying (and I think an ondisk ceph-osd > feature is enough here, then document that users should upgrade to > hammer 0.94.6 or infernalis 9.2.1 before moving to jewel). > > sage > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
