<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div>Thanks the reply, </div><div><br></div><div><span style="line-height: 23.7999992370605px;"> "why don¡¯t you just explicitly delete all the TreePatterns referenced from your container at that point? There would be no need to reference counting or cycle detection."</span></div><div><span style="line-height: 23.7999992370605px;"><br></span></div><div><span style="line-height: 23.7999992370605px;">It is a good suggestion, and using std::shared_pointer seems also possible.</span></div><div><span style="line-height: 23.7999992370605px;"><br></span></div><div><span style="line-height: 23.7999992370605px;">I will think about it!</span></div><div><span style="line-height: 23.7999992370605px;"><br></span></div><div></div><div id="divNeteaseMailCard"></div><br>ÔÚ 2014-09-19 00:04:00£¬"David Blaikie" <dblaikie@gmail.com> дµÀ£º<br> <blockquote id="isReplyContent" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Sep 7, 2014 at 4:48 PM, Andrew Trick <span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On Sep 2, 2014, at 8:08 PM, wuhui1973 <<a href="mailto:wuhui1973@163.com" target="_blank">wuhui1973@163.com</a>> wrote:</div><br><div><div style="font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;line-height:1.7;font-size:14px;font-family:Arial"><div>Hi Andrew:<br><br>Please find the anwser on line.<br><br><br></div><div></div><div></div><div><br></div>At 2014-09-03 10:54:21, "Andrew Trick" <<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>> wrote:<br><blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><br><blockquote type="cite"><div>On Sep 2, 2014, at 7:22 PM, wuhui1973 <<a href="mailto:wuhui1973@163.com" target="_blank">wuhui1973@163.com</a>> wrote:</div><br><div><div style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><div style="line-height:1.7;font-family:Arial;font-size:14px"><div><br>Hello Andrew, Hal:</div><div> </div><div>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.</div></div></div></div></blockquote><div><br></div><div>What does the dtor do beside release memory?</div><div> </div><div>[huiwu] it will invoke dtor of its member. For example, in TreePattern object, its dtor will invoke dtor of following members:</div><div> </div><div>Trees (type std::vector<TreePatternNode*>)</div><div>NamedNodes (type StringMap<SmallVector<TreePatternNode*,1> >)</div><div>Args (type std::vector<std::string>)</div><div> </div><div>So if dtor if TreePattern is not invoked, only just memory occupied by this TreePattern instance is freeed, memory allocated on heap by all these containers leak!<br></div></blockquote></div></div></blockquote><div><br></div></span><div>I see how it might be hard to allocate all the referenced data using BumpPtrAllocator. That¡¯s unfortunate, because it would be a good way to design this sort of thing.</div><div><br></div><div>Given that you will continue using normal heap allocation, there is still something I don¡¯t understand about the design. It looks like your adding references to all the nodes to a global container. So if there is one point in the code where no TreePatterns should be alive, such as the point where you would call ReclaimCycle, or destroy the BumpPtrAllocator, why don¡¯t you just explicitly delete all the TreePatterns referenced from your container at that point? There would be no need to reference counting or cycle detection.</div></div><div><br></div><div>[huiwu] Unfortunately, most of the pointers are also held by standard containers (i.e, std::vector, std::array)</div></div></blockquote><div><br></div><div>Yeah - I'm confused by the complexity and have the same question.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><br></div><div>-Andy</div><span class=""><div><br></div><br><blockquote type="cite"><div><div style="font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;line-height:1.7;font-size:14px;font-family:Arial"><blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><blockquote type="cite"><div><div style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><div style="line-height:1.7;font-family:Arial;font-size:14px"><div>And I also consider std::shared_ptr, but not use it for following findings:</div><div> </div><div>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.</div><div>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.</div></div></div></div></blockquote><div><br></div><div>Thanks.</div><div><br><blockquote type="cite"><div style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><div style="line-height:1.7;font-family:Arial;font-size:14px"><div> <span style="line-height:1.7">In attachment, there are two kinds of suffix:</span></div><div> </div><div>*.review is for review</div><div>*.fix is for submit</div></div></div></blockquote><div><br></div><div>You can run clang-format before your patch, submit that as a separate patch (no need for a separate review), then submit your patch on top of it.</div><div><br></div><div>-Andy</div><br><blockquote type="cite"><div style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><div style="line-height:1.7;font-family:Arial;font-size:14px"><div><span style="line-height:1.7"> </span></div></div></div></blockquote><blockquote type="cite"><div style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><div style="line-height:1.7;font-family:Arial;font-size:14px"><div>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<br><br><br></div><div></div><div></div><div><br></div>At 2014-08-07 11:59:44, "Andrew Trick" <<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>> wrote:<br><blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><br><div><blockquote type="cite"><div>On Aug 7, 2014, at 2:29 AM, wuhui1973 <<a href="mailto:wuhui1973@163.com" target="_blank">wuhui1973@163.com</a>> wrote:</div><br><div><span style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:23px;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;float:none;white-space:normal;display:inline!important">>> [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 </span><span style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal">between handling .td files</span><span style="font-style:normal;font-variant:normal;font-weight:normal;font-size:14px;line-height:1.7;font-family:Arial;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal">. 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?</span></div></blockquote></div><br><div>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.</div><div><br></div><div>-Andy</div></blockquote></div></div><br style="font-style:normal;font-variant:normal;font-weight:normal;font-size:12px;line-height:normal;font-family:Helvetica;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><br style="font-style:normal;font-variant:normal;font-weight:normal;font-size:12px;line-height:normal;font-family:Helvetica;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><span title="neteasefooter" style="font-style:normal;font-variant:normal;font-weight:normal;font-size:12px;line-height:normal;font-family:Helvetica;text-transform:none;text-indent:0px;letter-spacing:normal;word-spacing:0px;white-space:normal"><span></span></span><span><DAGISelMatcherGen.cpp.review></span><span><TableGen.cpp.fix></span><span><DAGISelMatcherGen.cpp.fix></span><span><CodeGenDAGPatterns.cpp.fix></span><span><CodeGenDAGPatterns.h.fix></span><span><CodeGenDAGPatterns.h.review></span><span><CodeGenDAGPatterns.cpp.review></span></blockquote></div></blockquote></div></div></blockquote></span></div><br></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div><br><br><span title="neteasefooter"><span id="netease_mail_footer"></span></span>