I believe it came in on https://github.com/apache/trafficserver/pull/3246
On Wed, Dec 12, 2018 at 7:04 PM Phil Sorber <sor...@apache.org> wrote: > Can you point me to the PR/commit on github? > > Thanks. > > On Wed, Dec 12, 2018 at 4:51 PM SUSAN HINRICHS <shinr...@ieee.org> wrote: > > > Fei committed the don't dump fix for jemalloc a few months back. > > > > On Wed, Dec 12, 2018, 4:46 PM Leif Hedstrom <zw...@apache.org wrote: > > > > > > > > > > > > On Dec 12, 2018, at 4:42 PM, Phil Sorber <sor...@apache.org> wrote: > > > > > > > > The problem was that we were using jemalloc as a drop in replacement > > for > > > > malloc/free. The DONT_DUMP issue is easy to address when using the > > > jemalloc > > > > APIs directly. Perhaps someone will make me a salami sandwich and I > > will > > > > write the patch over the holiday break. Whiskey wouldn't hurt either. > > > > > > > > > I will provide both. I’ll even tap into the expensive, one of a kind > > > Jameson irish whiskey I picked up in Cork. > > > > > > — Leif > > > > > > > > > > > > > > > On Wed, Dec 12, 2018 at 6:49 AM Alan Carroll > > > > <solidwallofc...@oath.com.invalid> wrote: > > > > > > > >> Pushkar - based on my understanding of Fei's experiments, the issue > > was > > > >> doing the DONT_DUMP marking, which would cause problems with > jemalloc. > > > >> That's part of the effort of getting jemalloc ready. > > > >> > > > >> Walt - I've read through your code and I don't really the benefits. > > It's > > > >> definitely cleaner code, but I don't see the implementation > advantage, > > > >> particularly with regard to reducing pressure on heaps. E.g. "f > there > > > are a > > > >> lot of smaller dynamic > > > >> objects with short lifetimes, it will reduce thread blocking on the > > heap > > > >> mutex" - I don't see that. With the current implementation, in those > > > >> circumstances there is extremely little pressure on the heap because > > the > > > >> objects are popping on and off thread local free lists and not > hitting > > > the > > > >> underlying allocation mechanism. > > > >> > > > >> Also, AFAICT from the code, objects that are bigger than a pointer > are > > > >> allocated directly from the heap. Given that few, if any, of the ATS > > > >> objects in question are that small, it seems like this effectively > > > disables > > > >> freelists. How is that better than just calling new and delete > > directly? > > > >> > > > >> Another major problem is that, due the current free list > > implementation, > > > >> there is not much concern about constructors and destructors and > > > depending > > > >> on those to keep object state correct is likely to be bug prone. > This > > is > > > >> going to be an issue for jemalloc as well, but if we're going to do > > the > > > >> work we should move all the way to no free lists at all. > > > >> > > > >> > > > >> On Tue, Dec 11, 2018 at 6:05 PM Bryan Call <bc...@apache.org> > wrote: > > > >> > > > >>> The freelist can be used with jemalloc, but the thought/theory is > > that > > > >> you > > > >>> can turn off the freelist and use jemalloc and get similar > > performance. > > > >>> This needs to be validated. > > > >>> > > > >>> -Bryan > > > >>> > > > >>>> On Dec 11, 2018, at 3:18 PM, Walt Karas <wka...@oath.com.INVALID> > > > >> wrote: > > > >>>> > > > >>>> I thought jemalloc is used as a drop-in replacement for the > standard > > > >> lib > > > >>>> heap functions / operators. So how can the freelist stuff not > work > > > >> with > > > >>> it? > > > >>>> > > > >>>> On Tue, Dec 11, 2018 at 4:48 PM Bryan Call <bc...@apache.org> > > wrote: > > > >>>> > > > >>>>> There is no point in cleaning up the code if the plan is to not > use > > > it > > > >>> and > > > >>>>> remove it from our codebase. Work should be done on proving that > > > >>> jemalloc > > > >>>>> is valid alternative. > > > >>>>> > > > >>>>> If jemalloc doesn’t prove to workout, then we might look at > > cleaning > > > >> up > > > >>>>> the freelist. > > > >>>>> > > > >>>>> -Bryan > > > >>>>> > > > >>>>>> On Dec 10, 2018, at 5:42 PM, Walt Karas <wka...@oath.com.INVALID > > > > > >>> wrote: > > > >>>>>> > > > >>>>>> As far as one can tell is a big limitation with code like: > > > >>>>>> > > > >>>>>> #if (defined(__i386__) || defined(__arm__) || defined(__mips__)) > > && > > > >>>>>>> (SIZEOF_VOIDP == 4) > > > >>>>>>> > > > >>>>>>> #define FREELIST_POINTER(_x) (_x).s.pointer > > > >>>>>>> > > > >>>>>>> #define FREELIST_VERSION(_x) (_x).s.version > > > >>>>>>> > > > >>>>>>> #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) \ > > > >>>>>>> > > > >>>>>>> (_x).s.pointer = _p; \ > > > >>>>>>> > > > >>>>>>> (_x).s.version = _v > > > >>>>>>> > > > >>>>>>> #elif TS_HAS_128BIT_CAS > > > >>>>>>> > > > >>>>>>> #define FREELIST_POINTER(_x) (_x).s.pointer > > > >>>>>>> > > > >>>>>>> #define FREELIST_VERSION(_x) (_x).s.version > > > >>>>>>> > > > >>>>>>> #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) \ > > > >>>>>>> > > > >>>>>>> (_x).s.pointer = _p; \ > > > >>>>>>> > > > >>>>>>> (_x).s.version = _v > > > >>>>>>> > > > >>>>>>> #elif defined(__x86_64__) || defined(__ia64__) || > > > >>> defined(__powerpc64__) > > > >>>>>>> || defined(__aarch64__) || defined(__mips64) > > > >>>>>>> > > > >>>>>>> #define FREELIST_POINTER(_x) \ > > > >>>>>>> > > > >>>>>>> ((void *)(((((intptr_t)(_x).data) << 16) >> 16) | > > > >>>>>>> (((~((((intptr_t)(_x).data) << 16 >> 63) - 1)) >> 48) << 48))) > // > > > >> sign > > > >>>>>>> extend > > > >>>>>>> > > > >>>>>>> #define FREELIST_VERSION(_x) (((intptr_t)(_x).data) >> 48) > > > >>>>>>> > > > >>>>>>> #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) (_x).data = > > > >>>>>>> ((((intptr_t)(_p)) & 0x0000FFFFFFFFFFFFULL) | (((_v)&0xFFFFULL) > > << > > > >>> 48)) > > > >>>>>>> > > > >>>>>>> #else > > > >>>>>>> > > > >>>>>>> #error "unsupported processor" > > > >>>>>>> > > > >>>>>>> #endif > > > >>>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> On Mon, Dec 10, 2018 at 5:02 PM Leif Hedstrom <zw...@apache.org > > > > > >>> wrote: > > > >>>>>> > > > >>>>>>> > > > >>>>>>> > > > >>>>>>>> On Dec 10, 2018, at 10:29 AM, SUSAN HINRICHS < > shinr...@ieee.org > > > > > > >>>>> wrote: > > > >>>>>>>> > > > >>>>>>>> Based on Fei's measurements, the ATS freelists provide no > > benefit > > > >>> over > > > >>>>>>>> jemalloc. We are now in a position to do larger tests over > our > > > >>>>>>> production > > > >>>>>>>> installs. > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> Agreed, that was generally what I noticed too, except, I could > > not > > > >> get > > > >>>>> ATS > > > >>>>>>> to be stable with just jemalloc. It’d eventually get unhappy, > > but I > > > >>>>> didn’t > > > >>>>>>> investigate further. But this is my point, lets focus the > efforts > > > on > > > >>>>> moving > > > >>>>>>> us forward, to jemalloc, and not mess around with freelist as > it > > > is, > > > >>>>>>> because it works fine as far as I can tell. > > > >>>>>>> > > > >>>>>>> — leif > > > >>>>>>> > > > >>>>>>> > > > >>>>> > > > >>>>> > > > >>> > > > >>> > > > >> > > > >> -- > > > >> *Beware the fisherman who's casting out his line in to a dried up > > > >> riverbed.* > > > >> *Oh don't try to tell him 'cause he won't believe. Throw some bread > to > > > the > > > >> ducks instead.* > > > >> *It's easier that way. *- Genesis : Duke : VI 25-28 > > > >> > > > > > > > > >
