[llvm-commits] [PATCH] Add an iplist::clearAndLeakNodesUnsafely() function, speed up Recyler:clear()

[Jakob Stoklund Olesen]

On Jan 4, 2013, at 2:05 PM, Chandler Carruth <chandlerc at google.com> wrote:

> Sorry for the delay, and thanks for correcting my understanding of your expected use case... I have a couple more questions below:
> 
> On Wed, Jan 2, 2013 at 1:54 PM, Jakob Stoklund Olesen <jolesen at apple.com> wrote:
> 
> On Jan 2, 2013, at 1:01 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
>> On Wed, Jan 2, 2013 at 10:38 AM, Jakob Stoklund Olesen <jolesen at apple.com> wrote:
> 
> 
> 
>> As a first step, I need to convince an iplist to let go of its contents without pulling every object into cache (and dirtying all the cache lines as a bonus!). The clearAndLeakNodesUnsafely() function added in the first patch does that.
>> 
>> I feel like I would like this more if it were more generically applied. What do you think about making this a compile time policy decision for the ilist?
>> 
>> I'm imagining a bool template parameter which essentially says "LeakSubobjects" and which is a compile error if used in conjunction with a value type with non-trivial destructor.
>> 
>> Then, clear(), erase(), replace(), and other methods on ilist which currently spend time walking the ilist and deallocating can all benefit from the fast-path.
> 
> That won't work. Look at how we define ilist_node_traits for MachineInstrs. The addNodeToList() and removeNodeFromList() hooks do useful work when updating the Parent pointer. Similarly, deleteNode() puts the object back in the Recycler for MachineInstrs:
> 
> void ilist_traits<MachineInstr>::deleteNode(MachineInstr* MI) {
>   assert(!MI->getParent() && "MI is still in a block!");
>   Parent->getParent()->DeleteMachineInstr(MI);
> }
> 
> This is desirable behavior, except immediately before deallocating the entire MachineFunction.
> 
> Ahh, ok, I was going down the wrong path here. Thanks for that.
> 
> It sounds like you really want to make the destructor if the ilist itself be the no-op? Is there ever a case where you want the no-op clear that you aren't about to destroy the ilist?
> 
> If I've understood that correctly, I still think making this a template policy is the right way to go, and check that policy in the destructor before clearing the ilist.

The current behavior of iplist::clear() and ~iplist are what we want.

For example, when removing a basic block from a function, we want all of the instructions in the basic block to be recycled.

> Anyways, don't get held up here, we can iterate on the exact design as needed in post-commit review.

Thanks,
/jakob
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130104/fcc5af90/attachment.html>