Fixed bugs in List. Removed unnecessary head node.

PODVector speed optimization.

Engine/Container/Allocator.cpp

@@ -25,13 +25,13 @@
 
 #include "stdio.h"
 
-Allocator* AllocatorGetBlock(Allocator* allocator, unsigned nodeSize, unsigned capacity)
+AllocatorBlock* AllocatorReserveBlock(AllocatorBlock* allocator, unsigned nodeSize, unsigned capacity)
 {
     if (!capacity)
         capacity = 1;
     
-    unsigned char* blockPtr = new unsigned char[sizeof(Allocator) + capacity * (sizeof(AllocatorNode) + nodeSize)];
-    Allocator* newBlock = reinterpret_cast<Allocator*>(blockPtr);
+    unsigned char* blockPtr = new unsigned char[sizeof(AllocatorBlock) + capacity * (sizeof(AllocatorNode) + nodeSize)];
+    AllocatorBlock* newBlock = reinterpret_cast<AllocatorBlock*>(blockPtr);
     newBlock->nodeSize_ = nodeSize;
     newBlock->capacity_ = capacity;
     newBlock->free_ = 0;
@@ -46,7 +46,7 @@ Allocator* AllocatorGetBlock(Allocator* allocator, unsigned nodeSize, unsigned c
     }
     
     // Initialize the nodes. Free nodes are always chained to the first (parent) allocator
-    unsigned char* nodePtr = blockPtr + sizeof(Allocator);
+    unsigned char* nodePtr = blockPtr + sizeof(AllocatorBlock);
     AllocatorNode* firstNewNode = reinterpret_cast<AllocatorNode*>(nodePtr);
     
     for (unsigned i = 0; i < capacity; ++i)
@@ -66,23 +66,23 @@ Allocator* AllocatorGetBlock(Allocator* allocator, unsigned nodeSize, unsigned c
     return newBlock;
 }
 
-Allocator* AllocatorInitialize(unsigned nodeSize, unsigned initialCapacity)
+AllocatorBlock* AllocatorInitialize(unsigned nodeSize, unsigned initialCapacity)
 {
-    Allocator* block = AllocatorGetBlock(0, nodeSize, initialCapacity);
+    AllocatorBlock* block = AllocatorReserveBlock(0, nodeSize, initialCapacity);
     return block;
 }
 
-void AllocatorUninitialize(Allocator* allocator)
+void AllocatorUninitialize(AllocatorBlock* allocator)
 {
     while (allocator)
     {
-        Allocator* next = allocator->next_;
+        AllocatorBlock* next = allocator->next_;
         delete[] reinterpret_cast<unsigned char*>(allocator);
         allocator = next;
     }
 }
 
-void* AllocatorGet(Allocator* allocator)
+void* AllocatorReserve(AllocatorBlock* allocator)
 {
     if (!allocator)
         return 0;
@@ -98,7 +98,7 @@ void* AllocatorGet(Allocator* allocator)
     
     // Free nodes have been exhausted. Allocate a new larger block
     unsigned newCapacity = (allocator->capacity_ + 1) >> 1;
-    Allocator* newBlock = AllocatorGetBlock(allocator, allocator->nodeSize_, newCapacity);
+    AllocatorBlock* newBlock = AllocatorReserveBlock(allocator, allocator->nodeSize_, newCapacity);
     allocator->capacity_ += newCapacity;
     
     // We should have new free node(s) chained
@@ -110,7 +110,7 @@ void* AllocatorGet(Allocator* allocator)
     return ptr;
 }
 
-void AllocatorFree(Allocator* allocator, void* ptr)
+void AllocatorFree(AllocatorBlock* allocator, void* ptr)
 {
     if ((!allocator) || (!ptr))
         return;

Engine/Container/Allocator.h

@@ -23,11 +23,13 @@
 
 #pragma once
 
-struct Allocator;
+#include <new>
+
+struct AllocatorBlock;
 struct AllocatorNode;
 
 /// Allocator memory block
-struct Allocator
+struct AllocatorBlock
 {
     /// Node data size
     unsigned nodeSize_;
@@ -36,7 +38,7 @@ struct Allocator
     /// First free node
     AllocatorNode* free_;
     /// Next allocator block
-    Allocator* next_;
+    AllocatorBlock* next_;
     /// Nodes follow
 };
 
@@ -45,14 +47,71 @@ struct AllocatorNode
 {
     /// Next free node
     AllocatorNode* next_;
-    /// Payload follows
+    /// Data follows
 };
 
 /// Initialize a fixed allocator with the allocation size and initial capacity
-Allocator* AllocatorInitialize(unsigned nodeSize, unsigned initialCapacity = 1);
+AllocatorBlock* AllocatorInitialize(unsigned nodeSize, unsigned initialCapacity = 1);
 /// Uninitialize a fixed allocator. Frees all blocks in the chain
-void AllocatorUninitialize(Allocator* allocator);
-/// Allocate a node. Reserves a new block if necessary
-void* AllocatorGet(Allocator* allocator);
+void AllocatorUninitialize(AllocatorBlock* allocator);
+/// Reserve a node. Creates a new block if necessary
+void* AllocatorReserve(AllocatorBlock* allocator);
 /// Free a node. Does not free any blocks
-void AllocatorFree(Allocator* allocator, void* ptr);
+void AllocatorFree(AllocatorBlock* allocator, void* ptr);
+
+/// Template allocator class
+template <class T> class Allocator
+{
+public:
+    /// Construct
+    Allocator(unsigned initialCapacity = 0) :
+        allocator_(0)
+    {
+        if (initialCapacity)
+            allocator_ = AllocatorInitialize(sizeof(T), initialCapacity);
+    }
+    
+    /// Destruct
+    ~Allocator()
+    {
+        AllocatorUninitialize(allocator_);
+    }
+    
+    /// Allocate and default-construct an object
+    T* Allocate()
+    {
+        if (!allocator_)
+            allocator_ = AllocatorInitialize(sizeof(T));
+        T* newObject = static_cast<T*>(AllocatorReserve(allocator_));
+        new(newObject) T();
+        
+        return newObject;
+    }
+    
+    /// Allocate and copy-construct an object
+    T* Allocate(const T& object)
+    {
+        if (!allocator_)
+            allocator_ = AllocatorInitialize(sizeof(T));
+        T* newObject = static_cast<T*>(AllocatorReserve(allocator_));
+        new(newObject) T(object);
+        
+        return newObject;
+    }
+    
+    /// Destruct and free an object
+    void Free(T* object)
+    {
+        (object)->~T();
+        AllocatorFree(allocator_, object);
+    }
+    
+private:
+    /// Prevent copy construction
+    Allocator(const Allocator<T>& rhs);
+    /// Prevent assignment
+    Allocator<T>& operator = (const Allocator<T>& rhs);
+    
+    /// Allocator block
+    AllocatorBlock* allocator_;
+};

Engine/Container/List.h

@@ -25,8 +25,6 @@
 
 #include "ListBase.h"
 
-#include <new>
-
 /// Linked list template class
 template <class T> class List : public ListBase
 {
@@ -117,28 +115,18 @@ public:
     List() :
         ListBase()
     {
-        // Allocate and link the head and tail nodes
-        allocator_ = AllocatorInitialize(sizeof(Node), 2);
-        head_ = AllocateNode();
-        tail_ = AllocateNode();
-        Node* head = GetHead();
-        Node* tail = GetTail();
-        head->next_ = tail;
-        tail->prev_ = head;
+        // Allocate the tail node
+        allocator_ = AllocatorInitialize(sizeof(Node));
+        head_ = tail_ = AllocateNode();
     }
     
     /// Construct from another list
     List(const List<T>& list) :
         ListBase()
     {
-        // Allocate and link the head and tail nodes
-        allocator_ = AllocatorInitialize(sizeof(Node), list.Size() + 2);
-        head_ = AllocateNode();
-        tail_ = AllocateNode();
-        Node* head = GetHead();
-        Node* tail = GetTail();
-        head->next_ = tail;
-        tail->prev_ = head;
+        // Allocate the tail node
+        allocator_ = AllocatorInitialize(sizeof(Node));
+        head_ = tail_ = AllocateNode();
         
         // Then assign the other list
         *this = list;
@@ -148,7 +136,6 @@ public:
     ~List()
     {
         Clear();
-        FreeNode(GetHead());
         FreeNode(GetTail());
         AllocatorUninitialize(allocator_);
     }
@@ -233,13 +220,7 @@ public:
         Node* destNode = static_cast<Node*>(dest.ptr_);
         Iterator it = start;
         while (it != end)
-        {
-            Iterator current = it++;
-            InsertNode(destNode, *it);
-            // Break if the iterator got stuck
-            if (it == current)
-                break;
-        }
+            InsertNode(destNode, *it++);
     }
     
     /// Erase the last node
@@ -252,10 +233,7 @@ public:
     /// Erase a node from the list. Return an iterator to the next element
     Iterator Erase(Iterator it)
     {
-        Iterator current = it++;
-        EraseNode(static_cast<Node*>(current.ptr_));
-        
-        return it;
+        return Iterator(EraseNode(static_cast<Node*>(it.ptr_)));
     }
     
     /// Erase a range of nodes from the list. Return an iterator to the next element
@@ -263,13 +241,7 @@ public:
     {
         Iterator it = start;
         while (it != end)
-        {
-            Iterator current = it++;
-            // Break if the iterator got stuck
-            if (it == current)
-                break;
-            EraseNode(static_cast<Node*>(current.ptr_));
-        }
+            it = EraseNode(static_cast<Node*>(current.ptr_));
         
         return it;
     }
@@ -278,13 +250,13 @@ public:
     void Clear()
     {
         while (size_)
-            EraseNode(GetTail()->GetPrev());
+            EraseNode(GetHead());
     }
     
-    /// Return iterator to the first actual node
-    Iterator Begin() { return Iterator(GetHead()->GetNext()); }
-    /// Return iterator to the first actual node
-    ConstIterator Begin() const { return ConstIterator(GetHead()->GetNext()); }
+    /// Return iterator to the first node
+    Iterator Begin() { return Iterator(GetHead()); }
+    /// Return iterator to the first node
+    ConstIterator Begin() const { return ConstIterator(GetHead()); }
     /// Return iterator to the end
     Iterator End() { return Iterator(GetTail()); }
     /// Return iterator to the end
@@ -307,37 +279,52 @@ private:
     /// Insert a value into the list
     void InsertNode(Node* dest, const T& value)
     {
-        // The head node can not be replaced
-        if ((!dest) || (dest == head_))
+        if (!dest)
             return;
         
         Node* newNode = AllocateNode(value);
+        Node* prev = dest->GetPrev();
         newNode->next_ = dest;
-        newNode->prev_ = dest->prev_;
+        newNode->prev_ = prev;
+        if (prev)
+            prev->next_ = newNode;
         dest->prev_ = newNode;
+        
+        
+        // Reassign the head node if necessary
+        if (dest == GetHead())
+            head_ = newNode;
+        
         ++size_;
     }
     
-    /// Erase a node from the list
-    void EraseNode(Node* toRemove)
+    /// Erase a node from the list. Return pointer to the next element, or to the end if could not erase
+    Node* EraseNode(Node* toRemove)
     {
-        // The head or tail can not be removed
-        if ((!toRemove) || (toRemove == head_) || (toRemove == tail_))
-            return;
+        // The tail node can not be removed
+        if ((!toRemove) || (toRemove == tail_))
+            return GetTail();
         
-        /// \todo Should check that the node belongs to the list
         Node* prev = toRemove->GetPrev();
         Node* next = toRemove->GetNext();
-        prev->next_ = next;
+        if (prev)
+            prev->next_ = next;
         next->prev_ = prev;
+        
+        // Reassign the head node if necessary
+        if (toRemove == GetHead())
+            head_ = next;
+        
         FreeNode(toRemove);
         --size_;
+        
+        return next;
     }
     
     /// Allocate a node
     Node* AllocateNode()
     {
-        Node* newNode = static_cast<Node*>(AllocatorGet(allocator_));
+        Node* newNode = static_cast<Node*>(AllocatorReserve(allocator_));
         new(newNode) Node();
         return newNode;
     }
@@ -345,7 +332,7 @@ private:
     /// Allocate a node with initial value
     Node* AllocateNode(const T& value)
     {
-        Node* newNode = static_cast<Node*>(AllocatorGet(allocator_));
+        Node* newNode = static_cast<Node*>(AllocatorReserve(allocator_));
         new(newNode) Node(value);
         return newNode;
     }

Engine/Container/ListBase.h

@@ -60,14 +60,14 @@ public:
     /// Go to the next node
     void GotoNext()
     {
-        if (ptr_->next_)
+        if (ptr_)
             ptr_ = ptr_->next_;
     }
     
     /// Go to the previous node
     void GotoPrev()
     {
-        if (ptr_->prev_)
+        if (ptr_)
             ptr_ = ptr_->prev_;
     }
     
@@ -100,7 +100,7 @@ protected:
     /// Tail node pointer
     ListNodeBase* tail_;
     /// Node allocator
-    Allocator* allocator_;
+    AllocatorBlock* allocator_;
     /// Number of nodes
     unsigned size_;
 };
@@ -141,14 +141,12 @@ class TreeIteratorBase
 public:
     TreeIteratorBase() :
         ptr_(0),
-        next_(0),
         prev_(0)
     {
     }
     
     TreeIteratorBase(TreeNodeBase* ptr) :
         ptr_(ptr),
-        next_(0),
         prev_(0)
     {
     }
@@ -162,13 +160,10 @@ public:
     {
         if (!ptr_)
         {
-            ptr_ = next_;
-            next_ = 0;
             prev_ = 0;
             return;
         }
         
-        next_ = 0;
         prev_ = ptr_;
         
         if (!ptr_->link_[1])
@@ -190,12 +185,10 @@ public:
         if (!ptr_)
         {
             ptr_ = prev_;
-            next_ = 0;
             prev_ = 0;
             return;
         }
         
-        next_ = ptr_;
         prev_ = 0;
         
         if (!ptr_->link_[0])
@@ -214,9 +207,7 @@ public:
     
     /// Current node pointer
     TreeNodeBase* ptr_;
-    /// Next node pointer
-    TreeNodeBase* next_;
-    /// Previous node pointer
+    /// Previous node pointer, needed to back from the end
     TreeNodeBase* prev_;
 };
 
@@ -268,7 +259,7 @@ protected:
     /// Root node
     TreeNodeBase* root_;
     /// Node allocator
-    Allocator* allocator_;
+    AllocatorBlock* allocator_;
     /// Number of nodes
     unsigned size_;
 };

Engine/Container/Map.h

@@ -26,8 +26,6 @@
 #include "ListBase.h"
 #include "Pair.h"
 
-#include <new>
-
 // Based on http://eternallyconfuzzled.com/tuts/datastructures/jsw_tut_rbtree.aspx
 
 /// Map template class using a red-black tree
@@ -188,6 +186,13 @@ public:
         return *this;
     }
     
+    /// Add-assign a map
+    Map<T, U>& operator += (const Map<T, U>& rhs)
+    {
+        Insert(rhs.Begin(), rhs.End());
+        return *this;
+    }
+    
     /// Test for equality with another map
     bool operator == (const Map<T, U>& rhs) const
     {
@@ -226,11 +231,17 @@ public:
         return false;
     }
     
-    /// Add-assign a map
-    Map<T, U>& operator += (const Map<T, U>& rhs)
+    /// Index the map. Create new node if key not found
+    U& operator [] (const T& key)
     {
-        Insert(rhs.Begin(), rhs.End());
-        return *this;
+        Node* node = FindNode(key);
+        if (node)
+            return node->pair_.second_;
+        else
+        {
+            node = InsertNode(key, U());
+            return node->pair_.second_;
+        }
     }
     
     /// Clear the map
@@ -290,19 +301,6 @@ public:
         return FindNode(key) != 0;
     }
     
-    /// Index the map. Create new node if key not found
-    U& operator [] (const T& key)
-    {
-        Node* node = FindNode(key);
-        if (node)
-            return node->pair_.second_;
-        else
-        {
-            node = InsertNode(key, U());
-            return node->pair_.second_;
-        }
-    }
-    
     /// Return iterator to the node with key, or end iterator if not found
     Iterator Find(const T& key) { Node* node = FindNode(key); return node ? Iterator(node) : End(); }
     /// Return const iterator to the node with key, or null iterator if not found
@@ -533,7 +531,7 @@ private:
     {
         if (!allocator_)
             allocator_ = AllocatorInitialize(sizeof(Node));
-        Node* newNode = static_cast<Node*>(AllocatorGet(allocator_));
+        Node* newNode = static_cast<Node*>(AllocatorReserve(allocator_));
         new(newNode) Node();
         return newNode;
     }
@@ -543,7 +541,7 @@ private:
     {
         if (!allocator_)
             allocator_ = AllocatorInitialize(sizeof(Node));
-        Node* newNode = static_cast<Node*>(AllocatorGet(allocator_));
+        Node* newNode = static_cast<Node*>(AllocatorReserve(allocator_));
         new(newNode) Node(key, value);
         return newNode;
     }

Engine/Container/PODVector.h

@@ -145,9 +145,11 @@ public:
     /// Add an element at the end
     void Push(const T& value)
     {
-        unsigned oldSize = size_;
-        Resize(size_ + 1);
-        GetBuffer()[oldSize] = value;
+        if (size_ < capacity_)
+            ++size_;
+        else
+            Resize(size_ + 1);
+        Back() = value;
     }
     
     /// Add another vector at the end

Engine/Container/Set.h

@@ -25,8 +25,6 @@
 
 #include "ListBase.h"
 
-#include <new>
-
 // Based on http://eternallyconfuzzled.com/tuts/datastructures/jsw_tut_rbtree.aspx
 
 /// Set template class using a red-black tree
@@ -194,8 +192,6 @@ public:
         return false;
     }
     
-
-    
     /// Clear the set
     void Clear()
     {
@@ -478,7 +474,7 @@ private:
     {
         if (!allocator_)
             allocator_ = AllocatorInitialize(sizeof(Node));
-        Node* newNode = static_cast<Node*>(AllocatorGet(allocator_));
+        Node* newNode = static_cast<Node*>(AllocatorReserve(allocator_));
         new(newNode) Node();
         
         return newNode;
@@ -489,7 +485,7 @@ private:
     {
         if (!allocator_)
             allocator_ = AllocatorInitialize(sizeof(Node));
-        Node* newNode = static_cast<Node*>(AllocatorGet(allocator_));
+        Node* newNode = static_cast<Node*>(AllocatorReserve(allocator_));
         new(newNode) Node(key);
         return newNode;
     }

Engine/Container/Vector.h

@@ -147,7 +147,6 @@ public:
     /// Add an element at the end
     void Push(const T& value)
     {
-        unsigned oldSize = size_;
         Resize(size_ + 1, &value);
     }
     
@@ -157,7 +156,6 @@ public:
         if (!vector.size_)
             return;
         
-        unsigned oldSize = size_;
         Resize(size_ + vector.size_, vector.GetBuffer());
     }