Solution for memory leak in tblgen

[wuhui1973]

ping


Hi Andrew and David:



The cyclic reference formed should be like this:


TreePatternNode  <------------------------------------------------------------------+
    PredicateFns (std::vector<TreePredicateFn>)                                   |
        PatFragRec ------> TreePattern                                                      |
                                         ^  Trees (std::vector<TreePatternNodePtr>) +------------> Other TreePatternNode
                                         |
Other TreePatternNode --+
  
In the design all these pointers are reference counted, and I have designed a "operator=" like this:


+  TreeNodePtr &operator=(const T *node) {
+    this->Release();
+    this->Member = const_cast<T *>(node);
+    this->AddRef();
+    return *this;
+  }


So if we assign "null" to "PatFragRec" or "Trees", for example "PatFragRec", if it finds it's the the last referrer, it will free the TreePattern pointed by
"PatFragRec", which in turn frees "Trees" which contains the TreePatternNode referring the TreePattern. So what we need to do is to assign all "PatFragRec" 
or "Trees" to "null".


So in fact, FreeCycleRef::run can be simplified as:


void FreeCycleRef::run() {
 std::vector<TreePatternNodePtr> tmp;  // temporarily holding PatFragRec to prevent TreePatternNode from deleting,
                                                                // which may erase some items from the container and break below for-loop
 for (TPContainerType::iterator it = TPContainer.begin(); it != TPContainer.end(); ++it)
 {
       const std::vector<TreePredicateFn> &vec = iter->second->getPredicateFns();
       for (unsigned i = 0, e = vec.size(); i < e; ++i) {
          TreePredicateFn *p = const_cast<TreePredicateFn *>(&vec[i]);
          tmp.pushback(p->PatFragRec);
          p->PatFragRec = nullptr;
      }
   }
} // all Tree Nodes should be released here!


However, I follow the cyclic chain and do the cleaning along the way just out of this consideration:


You see the algorithm to reclaim the cyclic reference is very special to current situation. In future, if the data layout changed, and 
the algorithm may don't work, it should be found out as quick as possible -- that is the purpose of the assertion at line 2417:


+    assert(refCnt == 0 && "The RefCnt diverged");






At 2014-09-23 06:04:25, "Andrew Trick" <atrick at apple.com> wrote:
>
>> On Sep 22, 2014, at 8:03 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>> If you haev cycles, reference counting is not the right tool.
>> 
>> If you are working around the limitations of reference counting because you have cycles by defining a cleanup pass - why not just do all the ownership and destruction there? I'm still not really following, but haven't taken the time to look into the code in more detail.
>
>I have the same question.
>Andy



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141110/7a864dd3/attachment.html>