Hi Jose, Thanks for reviewing!
On Thursday, July 17, 2014 16:01:50 Jose Fonseca wrote: > For patch 1 I'd prefer that instead of keeping a global list of > contexts, these are passed by the caller as argument to gallivm_create() > . It will be a more invasive change, but it will be cleaner. And in > particular it will garantee that when llvmpipe contexts are destroyed > there will be no lingering llvm contexts neither. Ok, the rationale to resort to the pool was for me that the EVT instances in this static map are in the end keyed beyond their type by their LLVMContext pointer value they were belonging to. That means there is a risc of having a LLVMContext that introduces such an EVT entry which is belonging to that LLVMContext and put into the global EVT map. Now assume the LLVMContext gets destroyed and a new one is created later at the same address than the previous one. Now the cached EVT is found but does not belong to the LLVMContext. Now I have no clue if this leads to real problems with these EVT entries nor do we have a huge probability that this really happens. Means that a next LLVMContext is malloced at the same address. But if this is a problem is probably very difficult to track. If you just want to have the LLVMContext instances cleaned up at module unloading so that valgrind runs or similar don't report leaks - we can dispose them at shared library unload. Ahh, I see we even should if we stick with a pool, the driver module might be loaded and unloaded several times which leaks these LLVMContexts with the current series. Also pooling them means that we are guaranteed to not accumulate these EVT instances in that LLVM internal cache as we get at most of them as we have gallivm_state instances around at a time. That means even an application which is repeatedly creating and destroying GL contexts can not go mad memory wise by introducing new EVT instances for LLVMContexts at different addresses. For my use case both above potential problems did never occur. So In the end I don't mind, but wanted to mention this. What do you think? Mathias _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
