[PATCH] D75281: BumpPtrAllocator: Use extern templates and out of line methods

[Reid Kleckner via Phabricator via llvm-commits]

rnk planned changes to this revision.
rnk added a comment.

In D75281#1896396 <https://reviews.llvm.org/D75281#1896396>, @hans wrote:

> I thought we wanted BumpPtrAllocatorImpl<>::Allocate to be inline though, so that Size and Alignment could often be constant folded, and so that the fast path (when the allocation fits in the current slab) becomes small and fast.
>
> Also, how does the extern template work here? It looks like you're declaring/defining it with the default template parameters. If someone instantiates it with other parameters, where will they get the Allocate() definition from? If it's never instantiated with the non-default params, maybe it shouldn't be a template in the first place -- this would be my preferred solution because it's remarkably complicated for a quick bump allocator.


Looking at it again, I think most of the expense comes from the "custom sized slab" handling. It calls `SmallVector<std::pair<void*, size_t>>::push_back`, which is probably where most of the instantiation time goes. If we could move that out of line and make it non-dependent, we'd be good to go.

Regarding the explicit instantiation, all of the methods are still defined in the header, so any users with non-default template args will instantiate their own definitions of Allocate & co. And I agree, this should be less templated, but last I checked, there are enough non-default users that it's not trivial. And 4096 is probably too small of a slab size, but that's a project for another day.

I think I'll play around with this some more to see if we can get the wins without blocking `Allocate` inlining. I'm not sure how much it matters, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75281/new/

https://reviews.llvm.org/D75281