[PATCH] D119177: [demangler] Fix buffer growth

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxxabi/src/demangle/Utility.h:43-45
+        constexpr size_t MinAlloc = 1024;
+        if (!BufferCapacity && Need < MinAlloc)
+          Need = MinAlloc;
----------------
urnathan wrote:
> ChuanqiXu wrote:
> > urnathan wrote:
> > > ChuanqiXu wrote:
> > > > It looks like that `MinInitAlloc` is a better name. I would like to move line 43~45 after line 38. Then we could simplify the code to:
> > > > ```
> > > > size_t Need = N + CurrentPosition;
> > > > /// A comment here
> > > > constexpr size_t MinInitAlloc = 1024;
> > > >  if (!BufferCapacity)
> > > >       Need = max(Need, MinInitAlloc);
> > > > if (Need > BufferCapacity)
> > > >        BufferCapacity = max(Need, BufferCapacity * 2);
> > > > ```
> > > I put the MinInitAlloc checking inside the outer if, to avoid making the non-allocating path more complex.  We only need to check the minium when we know we're going to allocate
> > Now it lost the check: `if (!BufferCapacity)`. Is it intentional? Now it wouldn't allocate at the initialization only. So the name may be not appropriate.
> that check is unnecessary, we'll only pick MinInitAlloc on the first allocation -- because later allocations will be for a greater amount.
Yeah, it is true. I didn't figure it out previously.


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

https://reviews.llvm.org/D119177