Ah, yes I recall now. Did we ever take the next step here like was mentioned in the comments?
On Wed, Dec 12, 2018 at 5:45 PM Susan Hinrichs <shinr...@apache.org> wrote: > 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 > > > > >> > > > > > > > > > > > > > >
