On Thu, 11 Nov 2004, Morten Ofstad wrote:> I guess the issue of singleton objects leaking is not something that he > LLVM team is very concerned about, since there has been no replies to my > first mail... But for me it is a big problem. I can't use LLVM in our > project if it triggers our leak detection system, since it will hide > other problems.Hi Morten, I appologize for not getting back to you sooner, I've been running behind on a project for a deadline thursday night.> Since actually deleting these objects is proving difficult (types can > have circular references, so you have to deal with things getting > deleted while there are still references to them), I propose to > introduce a new class SingletonPool to lib/Support which keeps all the > singleton instances and can be free'd without actually calling the > destructors, thus stopping our memory leak detection system from > complaining. All it would require is using new(SingletonPool) when > creating singleton objects. Is this an acceptable solution that you > would accept patches for?Can you sketch out a little bit more concretely what you're thinking? In particular, we really don't want to add any per-type or per-constant overhead (note that constants have the same issue you're running into with types), at least for a release build. Are you thinking of a map off to the side that just invokes all of the destructors of all types? -Chris -- http://llvm.org/ http://nondot.org/sabre/
>>Since actually deleting these objects is proving difficult (types can >>have circular references, so you have to deal with things getting >>deleted while there are still references to them), I propose to >>introduce a new class SingletonPool to lib/Support which keeps all the >>singleton instances and can be free'd without actually calling the >>destructors, thus stopping our memory leak detection system from >>complaining. All it would require is using new(SingletonPool) when >>creating singleton objects. Is this an acceptable solution that you >>would accept patches for?> Can you sketch out a little bit more concretely what you're thinking? In > particular, we really don't want to add any per-type or per-constant > overhead (note that constants have the same issue you're running into with > types), at least for a release build. Are you thinking of a map off to > the side that just invokes all of the destructors of all types?Well, I already tried that, but the destructors crash because they are referencing other things which are being destroyed - Constants are Users of each other and there is no easy way to destroy them in the right order. The same problem happens with the derived types because the reference counting is checking if the contained types are abstract when deleting them... No, what I was proposing with the mempool is (at exit) to free the memory all the constants and types live in without calling any destructors whatsoever. The overhead would be minimal, since we're just providing a special new operator that allocates the singletons in a known place, so no tracking has to be done per object. It would also solve another problem -- We generate new shader code when the user changes parameters, because the shader will be executed millions of times it makes sense to recompile it with changed constants to get maximum optimization. But if some of these parameters are floats, and there is no way to destroy constants in LLVM we have a serious memory leak situation here. Having the ability to flush all constants and types and clear the TypeMaps/ValueMaps would be very useful to us. Unless you have another suggestion for fixing this scenario I will go ahead and implement this solution - even if you don't want to take the patches for it, it's something I simply can't live without. Just to end on a positive note: It was very easy to generate the LLVM intermediate rep. from our code - I have simple code generation working already, with bindings to global data in our program and so on. If only I could sort out these memory leak problems everything would be very very sunny... m.
On Thu, 11 Nov 2004, Morten Ofstad wrote:> Well, I already tried that, but the destructors crash because they are > referencing other things which are being destroyed - Constants are Users > of each other and there is no easy way to destroy them in the right > order.There are ways around this, but it turns into a two-pass operation: loop over all constants to drop their uses, then loop over them to delete them all.> The same problem happens with the derived types because the > reference counting is checking if the contained types are abstract when > deleting them... No, what I was proposing with the mempool is (at exit) > to free the memory all the constants and types live in without calling > any destructors whatsoever. The overhead would be minimal, since we're > just providing a special new operator that allocates the singletons in a > known place, so no tracking has to be done per object.Ok, I'm not following then. You want to change the per-class operator new in the type and constant classes to avoid the VC tracking operator new? I'd like to find a way to do this that does not penalize non-VC users at all.> It would also solve another problem -- We generate new shader code when > the user changes parameters, because the shader will be executed > millions of times it makes sense to recompile it with changed constants > to get maximum optimization. But if some of these parameters are floats, > and there is no way to destroy constants in LLVM we have a serious > memory leak situation here. Having the ability to flush all constants > and types and clear the TypeMaps/ValueMaps would be very useful to us.If this is a serious issue, it would be straight-forward to add a global function to flush out unused constants. I would be happy to accept a patch to do that.> Unless you have another suggestion for fixing this scenario I will go > ahead and implement this solution - even if you don't want to take the > patches for it, it's something I simply can't live without.If you can come up with a solution for the first one (to satisfy your 'memory leak detector') that does not penalize users of other compilers, I would be happy to take the patch for it too.> Just to end on a positive note: It was very easy to generate the LLVM > intermediate rep. from our code - I have simple code generation working > already, with bindings to global data in our program and so on. If only > I could sort out these memory leak problems everything would be very > very sunny...Great!!! -Chris -- http://llvm.org/ http://nondot.org/sabre/