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 > >> > >