Solution for memory leak in tblgen

[wuhui1973]

ping





在 2014-09-04 11:54:33，"wuhui1973" <wuhui1973 at 163.com> 写道：









在 2014-09-03 12:20:59，"David Blaikie" <dblaikie at gmail.com> 写道：






On Tue, Sep 2, 2014 at 7:22 PM, wuhui1973 <wuhui1973 at 163.com> wrote:


Hello Andrew, Hal:
 
I have updated my solution. In these days, I have tried BumpPtrAlloc, but the biggest problem of it is BumpPtrAlloc won't invoke dtor before releasing the memory.


That's intentional - do the dtors do any useful work? (is there any reason the dtors of these things aren't trivial?)
[huiwu] Yes, the dtors aren't trivial.
 
And I also consider std::shared_ptr, but not use it for following findings:
 
1> Nodes of TreePatternNode & TreePattern form cycles in reference, I have designed a simple method to recycle them, which can't be used with shared_ptr.


Could you explain the method you used to resolve cycles and why it isn't possible with shared_ptr?
 
[huiwu] To collect cycles of reference, I defined a class FreeCycleRef which closely coupled with nodes classes.
 
> struct FreeCycleRef {
>   typedef std::map<unsigned, TreePatternNode *> TPNContainerType;
>   typedef std::map<unsigned, TreePattern *> TPContainerType;
>   // Containers used to remember all allocated objects. At the end of
>   // the pass, those remained should be cycle referred.
>   static TPNContainerType TPNContainer;  <--------------------- container for  nodes of TreePatternNode
>   static TPContainerType TPContainer;       <--------------------- container for nodes of TreePattern
>
>   static unsigned TPNUniqueId;                   <--------------------- Id for for nodes of TreePatternNode
>   static unsigned TPUniqueId;                      <--------------------- Id for nodes of TreePattern
>   static unsigned NumTPNAlloc;                  <--------------------- number of  nodes of TreePatternNode allocated
>   static unsigned NumTPAlloc;                     <--------------------- number of nodes of TreePattern allocated
>   static unsigned NumTPNRel;                     <--------------------- number of nodes of TreePatternNode released
>   static unsigned NumTPRel;                       <--------------------- number of nodes of TreePattern released

At creating node of TreePatternNode or TreePattern, the pointer will be saved in TPNContainer or TPContainer, while at releasing the node, the related pointer will be erased from the container. At the end of running, if there are any nodes left, they should be cyclic referred, to collect them:
 
> // This method breaks the cycle reference bewteen TreePatterNode &
> // TreePattern.
> void FreeCycleRef::run() {
>   DEBUG(errs() << TPNContainer.size() << " TPNs remain, " << TPContainer.size()
>                << " TPs remain\n");
>
>   while (!TPContainer.empty()) {                                            <--------------------- TreePattern node left
>     TPContainerType::iterator it = TPContainer.begin();
>     TreePattern *tmp = it->second;
>     assert(tmp && "This TPN is null");
>     assert(!tmp->IsFree() && "This TPN is not referred");
>
>     unsigned refCnt = tmp->getRefCnt();                               <----------------------- get the reference count of the node
>     {
>       // We temperarily increase this object's reference, so it will
>       // not be released during below processing.
>       TreePatternHolder tmpHolder(tmp);                               <------------------------ now the reference count incr by 1
>       // Check TPN for referring T*/P
>       for (TPNContainerType::iterator iter = TPNContainer.begin();     <--------------- loop the container, and find TreePatternNode node referring it
>            iter != TPNContainer.end(); ++iter) {
>         TreePatternNode *node = iter->second;
>         assert(node && "This TPN is null");
>         assert(!node->IsFree() && "This TPN is not referred");
>
>         const std::vector<TreePredicateFn> &vec = node->getPredicateFns();
>         for (unsigned i = 0, e = vec.size(); i < e; ++i) {
>           if (vec[i].PatFragRec == tmp) {                                              <----------------- TreePatternNode node that refers us!
>             const TreePredicateFn *cp = &vec[i];
>             TreePredicateFn *p = const_cast<TreePredicateFn *>(cp);
>             // By reset this field, the reference counter is decreased, but
>             // won't be 0, as we have increased it at beginning.
>             p->PatFragRec = nullptr;                                                   <------------------- free the referring TreePatternNode node
>             if (--refCnt == 0)                                                                 <------------------ loop exit point, but reference count in fact is 1
>               break;
>           }
>         }
>         if (refCnt == 0)                                                                       <------------------ loop exit point, but reference count in fact is 1
>           break;
>       }
>     }                                                                                                <-------------------- the TreePattern node is freed here, it will be erased from the container.
>     // We have found out all referrers, tmp should have been released now!
>     assert(refCnt == 0 && "The RefCnt diverged");                       <--------------------- sainty check to ensure everything goes smoothly
>   }
>   DEBUG(errs() << "Release done\n");
> }

(you know, if it's just a matter of needing intrusive reference counting (which I'd rather avoid if possible, but sometimes can't be helped) then that is possible with shared_ptr, usind std::enable_shared_from_this)
 
2> After adding the grammar sugar operator-> & operator T(), now most source files using TreePatternHolder & TreePatternNodeHolder now needn't be changed. We can do that because we check reference count on every reference counted objects in every related methods. So any abuse can be found earily as possible.
 
In attachment, there are two kinds of suffix:
 
*.review is for review
*.fix is for submit
 
The reason why need these files is because I had run clang-format on the related source files which results in too many unrelated differences





At 2014-08-07 11:59:44, "Andrew Trick" <atrick at apple.com> wrote:


On Aug 7, 2014, at 2:29 AM, wuhui1973 <wuhui1973 at 163.com> wrote:


>> [hui] I am new to tablgen, I hope I can explain it clearly. At some stage in building llvm, tablgen will compile .td file into .inc file (C++ source), it will compile .td file one after one till the stage is done. As now the tree nodes are just allocated but not freed, they get lost between handling .td files. So the patch aims to reclaim the memory when finishing one .td file. I think pool allocator can be an option, but as I know, pool allocator is just another sub-project under llvm, has it been built into llvm? And it needs be done by the DSA (data structure analysis) alogrithm?! Is there any example in llvm?


Sorry, I wasn’t clear what I was talking about. In LLVM, we have a class called BumpPtrAllocator. You can use that to allocate a set of nodes, then free them all between passes. If that accomplishes our goals for TableGen, then I think it’s the ideal solution.


-Andy



_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits






-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140918/3a8d3fa7/attachment.html>