4

I have a std::list of classes and want to remove entries that are marked for delete. I am using std::remove_if and erase.

class MyClass
{
    bool isDone(MyData& myData)
    {
        return myData.isDone();
    }

    void removeIfDone(std::list<MyData>& myList)
    {
        std::list<MyData>::iterator it =
            remove_if(myList.begin(), myList.end(), 
                  boost::bind(&MyClass::isDone, this, _1));
        myList.erase(it, myList.end());
    }
};

I am running on a small processor for which memory allocation and deallocation is very expensive. This remove is calling new and delete thousands of times in my application.

I have previously used boost::ref when passing a non-trivial variable as a bind parameter but in this case I think it is probably the creation and destruction of the functor itself or the copying of it which is causing the problem.

I would like to do something like

boost::bind(&MyClass::isDone, boost::ref(this), boost::ref(_1));

I can't find documentation on what is being created and destroyed. So my simple question is how do I make this more efficient?

Praetorian
  • 106,671
  • 19
  • 240
  • 328
Ant
  • 1,668
  • 2
  • 18
  • 35
  • have you checked if `std::vector>` or even `std::vector` isn't faster than `list`? Often it is – Andriy Tylychko Mar 24 '17 at 17:32
  • @andyt Thank you for the suggestion. That was 4 years ago and I don't work there any more but I'm fairly sure I was using a list because I needed the benefits of a list. – Ant Mar 28 '17 at 13:23

2 Answers2

9

Try replacing the call to std::remove_if with std::list::remove_if. The latter should only copy some pointers from the preceding and succeeding elements, instead of trying to move the elements to the end of the list, which is the cause of the multiple allocations you're seeing. An additional benefit is that since it is a member function of std::list, it actually removes (i.e. erases) the elements matching your criterion.

class MyClass
{
    bool isDone(MyData& myData)
    {
        return myData.isDone();
    }

    void removeIfDone(std::list<MyData>& myList)
    {
        myList.remove_if( boost::bind( &MyClass::isDone, this, _1 ) );
    }
};
Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • 2
    +1 This is the correct answer, `boost::bind` is completely unrelated. – ildjarn Mar 07 '13 at 20:41
  • Excellent answer. boost::bind() shouldn't affect speed at all (it's just syntactic sugar)--the real culprit is the algorithm itself, as you pointed out. – Drew Hall Mar 07 '13 at 20:44
  • On our system this made a factor of 10 difference. It's made me realise that functions in the library are not only generic - they work on a number of container types - but they manipulate the containers in a generic way which may not be the most efficient. Thank you everyone for your help. – Ant Mar 12 '13 at 14:42
0

You could perhaps solve your problem using a simple functor that you pass to your std::remove_if. This way you don't need to pass any args to it in the remove_if and save yourself a boost::bind

struct functor
{
public:
    bool operator()(MyData& mydata)
    { 
       return mydata.IsDone();
    }   
};

void removeIfDone(std::list<MyData>& myList)
{
   std::list<MyData>::iterator it =
     remove_if(myList.begin(), myList.end(), 
         functor()); //call operator() on functor
     myList.erase(it, myList.end());
}

An example that compiles here

Tony The Lion
  • 61,704
  • 67
  • 242
  • 415