On Fri, Dec 27, 2013 at 11:44 AM, Kostya Serebryany <[email protected]> wrote:
> > > > On Fri, Dec 27, 2013 at 11:39 AM, Chandler Carruth <[email protected]>wrote: > >> >> On Fri, Dec 27, 2013 at 2:10 AM, Kostya Serebryany <[email protected]>wrote: >> >>> + // This function may be called only a small fixed amount of times per >>> each >>> + // invocation, otherwise we do actually have a leak which we want to >>> report. >>> + // If this function is called more than kGraveYardMaxSize times, the >>> pointers >>> + // will not be properly buried and a leak detector will report a >>> leak, which >>> + // is what we want in such case. >>> >> >> Interesting. I didn't realize it was going to be *that* tightly bounded. >> >> This makes me wonder if the whole disable free thing should just be >> removed at this point. >> > I am all for it, --disable-free looks like a hack and we did not see any > compile-time loss from removing it (measure on bootstrap). > I take these my words back. Building 483.xalancbmk with -O0 (make -j32) shows around 0.5%-1% compile-time difference: with -disable-free : TIME: real: 8.140; user: 177.970; system: 13.180 TIME: real: 8.070; user: 177.940; system: 13.090 TIME: real: 8.103; user: 177.960; system: 13.410 TIME: real: 8.122; user: 178.110; system: 13.180 TIME: real: 8.093; user: 177.660; system: 13.210 without -disable-free : TIME: real: 8.140; user: 179.750; system: 13.260 TIME: real: 8.113; user: 178.770; system: 13.710 TIME: real: 8.101; user: 179.830; system: 13.180 TIME: real: 8.262; user: 180.160; system: 12.760 TIME: real: 8.125; user: 179.510; system: 13.070 0.5% may be worth this relatively innocent hack. > > >> Back in the day, we didn't so carefully use a BumpPtrAllocator in the >> ASTContext. Today, we might be fine to call free 10 times. >> > > But this is not 10 calls to free. This is 10-ish calls to delete, where > the DTOR destructs other objects and so on. It may call much more than 10 > frees. > Still, see above. > > >> Anyways, not trying to shift the goal posts; I mostly wanted to mention >> it in case someone gets some time to look into removing all of the leak >> stuff. >> >> >>> + static const size_t kGraveYardMaxSize = 16; >>> + static const void *GraveYard[kGraveYardMaxSize]; >>> + static llvm::sys::cas_flag GraveYardSize; >>> >> >> Do these being function-local statics defeat the purpose of using a basic >> atomic increment? I can't recall if we correctly eliminate the cxa_guard in >> these cases... >> > We will not have cxa_guard here because there is no dynamic init -- both > objects (the size and the array) are linker-initialized. > Atomic increment is here for the future case when clang becomes > multi-threaded. > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
