[Mesa-dev] [PATCH 05/12] gallivm: Use LLVM global context.

[Mathias Fröhlich]

Hi,

On Thursday, May 22, 2014 03:38:12 Jose Fonseca wrote:
> Sorry for the delay.
Mostly the same here.
So don't worry!
I just want this to work somehow and I think the problem is already too long standing ...
And the longer I think about that the longer I believe that we have already discussed
this and I initially did not even remember ...

> This was with some internal code (it was not even GL.)  But I've distilled the test case to its core essence.  It is in this git repos
> 
>   git clone git://people.freedesktop.org/~jrfonseca/llvm-jitstress
> 
> It's really simple to use.  If you follow the instructions there you'll be able easily to repro the problem and see precisely the same stack traces.
> 
> There's still some tweaks I want to do, but I hope to get this upstreamed into LLVM eventually.

Ok, I have not yet time to look at that, but I will!

> 
> > 
> > And additionally looking at this below backtrace snippet and analyzing
> > the code that is called in llvm:
> > 
> > On Wednesday, May 14, 2014 02:45:26 you wrote:
> > > The problem is that when I do this, memory starts leaking for every
> > > compilation (even with LLVM 3.4):
> > > 
> > >   -> 100.00% (672B): operator new(unsigned long)
> > >     -> 100.00% (672B): std::_Rb_tree<llvm::EVT, llvm::EVT,
> > >     std::_Identity<llvm::EVT>, llvm::EVT::compareRawBits,
> > >     std::allocator<llvm::EVT> >::_M_insert_unique(llvm::EVT const&)
> > >       -> 100.00% (672B): llvm::SDNode::getValueTypeList(llvm::EVT)
> > >         -> 100.00% (672B): llvm::SelectionDAG::getVTList(llvm::EVT)
> > 
> > Ok, I see, this std::set in ./lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> > holds EVT instances which have a possible pointer to
> > llvm::Type objects declared in llvm/IR/Type.h. And such a Type seems to
> > reference
> > a LLVMContext with the 'Context' member in llvm::Type.
> > Since the std::set<EVT, ...>'s less operator treats EVT's to equivalent EVT's
> > referencing llvm::Types originating from different LLVMContext instances
> > differently,
> > this grows with the LLVMContext instances around.
> > 
> > Ok, a correct solution inside llvm would be to prune these set entries once
> > the
> > LLVMContext dies.
> > 
> > A workaround from our side could probably be to have a LLVMContext private to
> > the
> > GL context as already suggested. That would still grow with the number of GL
> > contexts.
> 
> Yes, I think that's a great idea.
> 
> In short, besides the existing gallivm_context (which is actually not per context, but rather per module/ compilation unit) there should be a new object, that's truly per context, that holds two things:
> 
>  - LLVMContextRef
>  - src/gallium/auxiliary/gallivm/lp_bld_misc.cpp's ShaderMemoryManager.
> 
> Each draw_context and llvmpipe_context should one of these (it's fine not to share the same.
> 
> The existing gallivm_context should probably be renamed gallivm_module.
Ok, lets see where we end I might come back to that.


Anyway, the observation I posted last and the week thinking about that makes me believe that
the caching that is done by LLVM and its type cache might even break badly with an ABA type 
problem if a LLVMContext is deleted and a new one is created at the same address. Because the
llvm types cached there are keyed by their type and their LLVMContext address they belong to.
I have not really looked into if this would just work (then probably by accident) or if there
are deeper dependencies into the LLVMContext they belong to (think of possible back and forward
pointers from and to the context or to the cached objects).

So in the very end your initial comment seems the easiest fix . I mean the
thread local LLVMContext. The ABA problem cannot happen since the contexts are as suggested
never deleted. But a possible application could still fire with a lot of new created threads and
leak in the same way that you observe - again only per created thread.

The safe method would be to pool these contexts in a list containing all ever created LLVMContexts.
The list access to get a LLVMContext before use and return it afterwards must be guarded by a mutex then.
but then we will not leak and do not run into the LLVMContext address reuse ABA problem since the
initial LLVMContext stays alive until the end of the application.

Greetings

Mathias