Commit 573ae504 authored by Julien Finet's avatar Julien Finet
Browse files

Speedup vtkCollection::RemoveItem(vtkObject*)

To remove an item from its object, the list was browsed twice:
once in vtkCollection::RemoveItem(vtkObject*) to find the object index and
once in vtkCollection::RemoveItem(int) to remove the element.

Change-Id: I66775509aec36eda435b95bbbe3c94a6aa3ef586
parent 84917251
......@@ -12,6 +12,7 @@ create_test_sourcelist(Tests ${vtk-module}CxxTests.cxx
TestArraySize.cxx
TestArrayUserTypes.cxx
TestArrayVariants.cxx
TestCollection.cxx
TestConditionVariable.cxx
# TestCxxFeatures.cxx # This is in its own exe too.
TestDataArray.cxx
......
/*=========================================================================
Program: Visualization Toolkit
Module: TestCollection.cxx
Copyright (c) Ken Martin, Will Schroeder, Bill Lorensen
All rights reserved.
See Copyright.txt or http://www.kitware.com/Copyright.htm for details.
This software is distributed WITHOUT ANY WARRANTY; without even
the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE. See the above copyright notice for more information.
=========================================================================*/
#include "vtkCollection.h"
#include "vtkIntArray.h"
#include "vtkNew.h"
#include "vtkSmartPointer.h"
bool TestRegister();
bool TestRemoveItem(int index, bool removeIndex);
int TestCollection(int,char *[])
{
bool res = true;
res = TestRegister() && res;
res = TestRemoveItem(0, false) && res;
res = TestRemoveItem(1, false) && res;
res = TestRemoveItem(5, false) && res;
res = TestRemoveItem(8, false) && res;
res = TestRemoveItem(9, false) && res;
res = TestRemoveItem(0, true) && res;
res = TestRemoveItem(1, true) && res;
res = TestRemoveItem(5, true) && res;
res = TestRemoveItem(8, true) && res;
res = TestRemoveItem(9, true) && res;
return res ? EXIT_SUCCESS : EXIT_FAILURE;
}
bool IsEqual(vtkCollection* collection, const std::vector<vtkSmartPointer<vtkIntArray> >& v)
{
if (collection->GetNumberOfItems() != v.size())
{
return false;
}
vtkIntArray* dataArray = 0;
vtkCollectionSimpleIterator it;
int i = 0;
for (collection->InitTraversal(it);
(dataArray = vtkIntArray::SafeDownCast(collection->GetNextItemAsObject(it))) ; ++i)
{
if (v[i] != dataArray)
{
return false;
}
}
return true;
}
bool TestRegister()
{
vtkNew<vtkCollection> collection;
vtkIntArray* object = vtkIntArray::New();
collection->AddItem(object);
object->Delete();
if (object->GetReferenceCount() != 1)
{
std::cout << object->GetReferenceCount() << std::endl;
return false;
}
object->Register(0);
collection->RemoveItem(object);
if (object->GetReferenceCount() != 1)
{
std::cout << object->GetReferenceCount() << std::endl;
return false;
}
object->UnRegister(0);
return true;
}
bool TestRemoveItem(int index, bool removeIndex)
{
vtkNew<vtkCollection> collection;
std::vector<vtkSmartPointer<vtkIntArray> > objects;
for (int i = 0; i < 10; ++i)
{
vtkNew<vtkIntArray> object;
collection->AddItem(object.GetPointer());
objects.push_back(object.GetPointer());
}
if (removeIndex)
{
collection->RemoveItem(index);
}
else
{
vtkObject* objectToRemove = objects[index];
collection->RemoveItem(objectToRemove);
}
objects.erase(objects.begin() + index);
if (!IsEqual(collection.GetPointer(), objects))
{
std::cout << "TestRemoveItem failed:" << std::endl;
collection->Print(std::cout);
return false;
}
return true;
}
......@@ -36,16 +36,7 @@ vtkCollection::vtkCollection()
// objects from the collection.
vtkCollection::~vtkCollection()
{
vtkCollectionElement *elem;
while (this->NumberOfItems )
{
elem = this->Top;
this->Top = elem->Next;
this->Current = elem->Next;
this->NumberOfItems--;
this->DeleteElement(elem);
}
this->RemoveAllItems();
}
// protected function to delete an element. Internal use only.
......@@ -58,6 +49,33 @@ void vtkCollection::DeleteElement(vtkCollectionElement *e)
delete e;
}
// protected function to remove an element. Internal use only.
void vtkCollection::
RemoveElement(vtkCollectionElement *elem, vtkCollectionElement *prev)
{
if (prev)
{
prev->Next = elem->Next;
}
else
{
this->Top = elem->Next;
}
if (!elem->Next)
{
this->Bottom = prev;
}
if ( this->Current == elem )
{
this->Current = elem->Next;
}
this->NumberOfItems--;
this->DeleteElement(elem);
}
// Add an object to the list. Does not prevent duplicate entries.
void vtkCollection::AddItem(vtkObject *a)
{
......@@ -139,25 +157,24 @@ void vtkCollection::InsertItem(int i, vtkObject *a)
// in description of RemoveItem(int).
void vtkCollection::RemoveItem(vtkObject *a)
{
int i;
vtkCollectionElement *elem;
if (!this->Top)
{
return;
}
elem = this->Top;
for (i = 0; i < this->NumberOfItems; i++)
vtkCollectionElement *prev = NULL;
vtkCollectionElement *elem = this->Top;
for (int i = 0; i < this->NumberOfItems; i++)
{
if (elem->Item == a)
{
this->RemoveItem(i);
this->RemoveElement(elem, prev);
this->Modified();
return;
}
else
{
prev = elem;
elem = elem->Next;
}
}
......@@ -166,21 +183,15 @@ void vtkCollection::RemoveItem(vtkObject *a)
// Remove all objects from the list.
void vtkCollection::RemoveAllItems()
{
vtkCollectionElement *elem;
// Don't modify if collection is empty
if(this->NumberOfItems == 0)
{
return;
}
while (this->NumberOfItems )
while (this->NumberOfItems)
{
elem = this->Top;
this->Top = elem->Next;
this->Current = elem->Next;
this->NumberOfItems--;
this->DeleteElement(elem);
this->RemoveElement(this->Top, NULL);
}
this->Modified();
......@@ -317,28 +328,7 @@ void vtkCollection::RemoveItem(int i)
elem = elem->Next;
}
// j == i
if (prev)
{
prev->Next = elem->Next;
}
else
{
this->Top = elem->Next;
}
if (!elem->Next)
{
this->Bottom = prev;
}
if ( this->Current == elem )
{
this->Current = elem->Next;
}
this->NumberOfItems--;
this->DeleteElement(elem);
this->RemoveElement(elem, prev);
}
vtkCollectionIterator* vtkCollection::NewIterator()
......
......@@ -137,6 +137,8 @@ protected:
vtkCollection();
~vtkCollection();
virtual void RemoveElement(vtkCollectionElement *element,
vtkCollectionElement *previous);
virtual void DeleteElement(vtkCollectionElement *);
int NumberOfItems;
vtkCollectionElement *Top;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment