[Mesa-dev] [PATCH 05/12] gallivm: Use LLVM global context.
Jose Fonseca
jfonseca at vmware.com
Wed May 14 02:45:26 PDT 2014
----- Original Message -----
>
> Hi Jose,
>
> On Tuesday, May 13, 2014 08:16:34 Jose Fonseca wrote:
> > I tweaked the comment (was below) but indeed did a poor job. I think I'll
> > just put the original back:
> >
> > "We must never free LLVM contexts, because LLVM has several global
> > caches
> > which pointing/derived from objects owned by the context, causing false
> > memory leaks and false cache hits when these objects are destroyed."
>
> I have not actually tested your current patch series,
> but I am wondering if you expect this to be thread safe then
> in the sense that multiple compilations from disjoint OpenGL
> contexts running in the same application in different but
> concurrent threads are run?
>
> Up to now this is a problem that strikes me with an application here.
> For example every openscenegraph viewer based application driving
> multiple contexts from a single program suffers from this problem
> as they are just crashing on startup when multiple contexts
> in their threads compile their shaders. Flightgear is one such example.
> Also I know a guy who is working in medical visualization who is
> frequently asking me about llvmpipe getting thread safe in this sense.
>
> Given the documentation about LLVM contexts I would expect this to
> crash?
>
> I have provided a patch some time ago that uses a struct gallivm_state
> private allocated LLVM context. That one runs fine with the applications
> in question and also it survived a piglit run without regressions.
> Did you see this review request?
>
> So, I really would like to have this problem solved.
>
> thanks
>
> Mathias
Hi Mathias,
No, you're right, this won't be thread safe. The fact that LLVM internally also uses its global context even when we use private contexts means that fixing this probably require changes to LLVM itself though.
Thread safety hasn't been an immediately for us, but I agree it is important for several apps out there, and we benefit from all the testing llvmpipe gets. So it's worthwhile addressing it.
I actually missed your patch. Apologies.
If you rebase it on top of my series it can be reduced to merely this:
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_init.c b/src/gallium/auxiliary/gallivm/lp_bld_init.c
index a39c1a3..3d3d590 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_init.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_init.c
@@ -76,7 +76,7 @@ void LLVMLinkInMCJIT();
* TODO: For thread safety on multi-threaded OpenGL we should use one LLVM
* context per thread, and put them in a pool when threads are destroyed.
*/
-#define USE_GLOBAL_CONTEXT 1
+#define USE_GLOBAL_CONTEXT 0
#ifdef DEBUG
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)
-> 50.00% (336B): llvm::SelectionDAG::getVTList(llvm::EVT const*, unsigned int)
| -> 50.00% (336B): llvm::SelectionDAGBuilder::visitLoad(llvm::LoadInst const&)
| -> 50.00% (336B): llvm::SelectionDAGBuilder::visit(unsigned int, llvm::User const&)
| -> 50.00% (336B): llvm::SelectionDAGBuilder::visit(llvm::Instruction const&)
| -> 50.00% (336B): llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::Instruction const>, llvm::ilist_iterator<llvm::Instruction const>, bool&)
| -> 50.00% (336B): llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&)
| -> 50.00% (336B): llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)
| -> 50.00% (336B): llvm::FPPassManager::runOnFunction(llvm::Function&)
| -> 50.00% (336B): llvm::FunctionPassManagerImpl::run(llvm::Function&)
| -> 50.00% (336B): llvm::FunctionPassManager::run(llvm::Function&)
| -> 50.00% (336B): llvm::JIT::jitTheFunction(llvm::Function*, llvm::MutexGuard const&)
| -> 50.00% (336B): llvm::JIT::runJITOnFunctionUnlocked(llvm::Function*, llvm::MutexGuard const&)
| -> 50.00% (336B): llvm::JIT::getPointerToFunction(llvm::Function*)
| -> 50.00% (336B): llvm::ExecutionEngine::getPointerToGlobal(llvm::GlobalValue const*)
| -> 50.00% (336B): gallivm_jit_function [mesa/src/gallium/auxiliary/gallivm/lp_bld_init.c:596]
| -> 50.00% (336B): draw_llvm_create_variant [mesa/src/gallium/auxiliary/draw/draw_llvm.c:552]
| -> 50.00% (336B): llvm_middle_end_prepare [mesa/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:258]
| -> 50.00% (336B): vsplit_prepare [mesa/src/gallium/auxiliary/draw/draw_pt_vsplit.c:265]
| -> 50.00% (336B): draw_pt_arrays [mesa/src/gallium/auxiliary/draw/draw_pt.c:137]
| -> 50.00% (336B): draw_vbo [mesa/src/gallium/auxiliary/draw/draw_pt.c:547]
| -> 50.00% (336B): llvmpipe_draw_vbo [mesa/src/gallium/drivers/llvmpipe/lp_draw_arrays.c:131]
|
-> 50.00% (336B): llvm::SelectionDAG::getNode(unsigned int, llvm::DebugLoc, llvm::EVT, llvm::SDValue)
-> 50.00% (336B): llvm::SelectionDAGBuilder::visitBitCast(llvm::User const&)
-> 50.00% (336B): llvm::SelectionDAGBuilder::visit(unsigned int, llvm::User const&)
-> 50.00% (336B): llvm::SelectionDAGBuilder::visit(llvm::Instruction const&)
-> 50.00% (336B): llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::Instruction const>, llvm::ilist_iterator<llvm::Instruction const>, bool&)
-> 50.00% (336B): llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&)
-> 50.00% (336B): llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&)
-> 50.00% (336B): llvm::FPPassManager::runOnFunction(llvm::Function&)
-> 50.00% (336B): llvm::FunctionPassManagerImpl::run(llvm::Function&)
-> 50.00% (336B): llvm::FunctionPassManager::run(llvm::Function&)
-> 50.00% (336B): llvm::JIT::jitTheFunction(llvm::Function*, llvm::MutexGuard const&)
-> 50.00% (336B): llvm::JIT::runJITOnFunctionUnlocked(llvm::Function*, llvm::MutexGuard const&)
-> 50.00% (336B): llvm::JIT::getPointerToFunction(llvm::Function*)
-> 50.00% (336B): llvm::ExecutionEngine::getPointerToGlobal(llvm::GlobalValue const*)
-> 50.00% (336B): gallivm_jit_function [mesa/src/gallium/auxiliary/gallivm/lp_bld_init.c:596]
-> 50.00% (336B): generate_variant [mesa/src/gallium/drivers/llvmpipe/lp_state_fs.c:2629]
-> 50.00% (336B): llvmpipe_update_fs [mesa/src/gallium/drivers/llvmpipe/lp_state_fs.c:3162]
-> 50.00% (336B): llvmpipe_update_derived [mesa/src/gallium/drivers/llvmpipe/lp_state_derived.c:188]
-> 50.00% (336B): llvmpipe_draw_vbo [mesa/src/gallium/drivers/llvmpipe/lp_draw_arrays.c:69]
In other words, we'd be introducing leaks for sake of thread-safety. I care more for memory leaks than thread safety, but I can easily flip USE_GLOBAL_CONTEXT internally.
I'm actually working on a test tool for LLVM that allows to easily detect and diagnose this sort of leaks (they tend to be not ordinary leaks, but more infinitely growing pools of memory which does get free on program shutdown). I hope to propose the test tool to LLVM upstream, so that they can more efficiently track and address. And hopefully they can get this sort of issues sorted.
For now I'm going to commit my patch series as is, as I want feedback if it causes any problems, as it is now. After I commit the series, if you could verify that switching USE_GLOBAL_CONTEXT to 0 fixes multi-threading for you, then I'd be happy to commit it after a few weeks.
Jose
More information about the mesa-dev
mailing list