Merge pull request #1548 from mgreter/bugfix/memory-alignment

Fix and prepare memory manager for reference counting

Author: mgreter

src/ast.cpp

@@ -585,7 +585,7 @@ namespace Sass {
   }
 
   // create complex selector (ancestor of) from compound selector
-  Complex_Selector* Compound_Selector::to_complex(Memory_Manager<AST_Node>& mem)
+  Complex_Selector* Compound_Selector::to_complex(Memory_Manager& mem)
   {
     // create an intermediate complex selector
     return SASS_MEMORY_NEW(mem, Complex_Selector,

src/ast.hpp

@@ -66,7 +66,7 @@ namespace Sass {
   //////////////////////////////////////////////////////////
   // Abstract base class for all abstract syntax tree nodes.
   //////////////////////////////////////////////////////////
-  class AST_Node {
+  class AST_Node : public Memory_Object {
     ADD_PROPERTY(ParserState, pstate)
   public:
     AST_Node(ParserState pstate)
@@ -2040,7 +2040,7 @@ namespace Sass {
       return length() == 1 && (*this)[0]->is_universal();
     }
 
-    Complex_Selector* to_complex(Memory_Manager<AST_Node>& mem);
+    Complex_Selector* to_complex(Memory_Manager& mem);
     Compound_Selector* unify_with(Compound_Selector* rhs, Context& ctx);
     // virtual Selector_Placeholder* find_placeholder();
     virtual bool has_parent_ref();

src/context.cpp

@@ -51,7 +51,7 @@ namespace Sass {
   Context::Context(Context::Data initializers)
   : // Output(this),
     head_imports(0),
-    mem(Memory_Manager<AST_Node>()),
+    mem(Memory_Manager()),
     c_options               (initializers.c_options()),
     c_compiler              (initializers.c_compiler()),
     source_c_str            (initializers.source_c_str()),

src/context.hpp

@@ -26,7 +26,7 @@ namespace Sass {
   class Context {
   public:
     size_t head_imports;
-    Memory_Manager<AST_Node> mem;
+    Memory_Manager mem;
 
     struct Sass_Options* c_options;
     struct Sass_Compiler* c_compiler;

src/environment.hpp

@@ -17,7 +17,7 @@ namespace Sass {
     ADD_PROPERTY(Environment*, parent)
 
   public:
-    Memory_Manager<AST_Node> mem;
+    Memory_Manager mem;
     Environment();
     Environment(Environment* env);
     Environment(Environment& env);

src/eval.cpp

@@ -1162,7 +1162,7 @@ namespace Sass {
     return *l < *r;
   }
 
-  Value* Eval::op_numbers(Memory_Manager<AST_Node>& mem, enum Sass_OP op, const Number& l, const Number& r, bool compressed, int precision)
+  Value* Eval::op_numbers(Memory_Manager& mem, enum Sass_OP op, const Number& l, const Number& r, bool compressed, int precision)
   {
     double lv = l.value();
     double rv = r.value();
@@ -1213,7 +1213,7 @@ namespace Sass {
     return v;
   }
 
-  Value* Eval::op_number_color(Memory_Manager<AST_Node>& mem, enum Sass_OP op, const Number& l, const Color& rh, bool compressed, int precision)
+  Value* Eval::op_number_color(Memory_Manager& mem, enum Sass_OP op, const Number& l, const Color& rh, bool compressed, int precision)
   {
     Color r(rh);
     r.disp("");
@@ -1247,7 +1247,7 @@ namespace Sass {
     return SASS_MEMORY_NEW(mem, Color, rh);
   }
 
-  Value* Eval::op_color_number(Memory_Manager<AST_Node>& mem, enum Sass_OP op, const Color& l, const Number& r, bool compressed, int precision)
+  Value* Eval::op_color_number(Memory_Manager& mem, enum Sass_OP op, const Color& l, const Number& r, bool compressed, int precision)
   {
     double rv = r.value();
     if (op == Sass_OP::DIV && !rv) error("division by zero", r.pstate());
@@ -1259,7 +1259,7 @@ namespace Sass {
                            l.a());
   }
 
-  Value* Eval::op_colors(Memory_Manager<AST_Node>& mem, enum Sass_OP op, const Color& l, const Color& r, bool compressed, int precision)
+  Value* Eval::op_colors(Memory_Manager& mem, enum Sass_OP op, const Color& l, const Color& r, bool compressed, int precision)
   {
     if (l.a() != r.a()) {
       error("alpha channels must be equal when combining colors", r.pstate());
@@ -1275,7 +1275,7 @@ namespace Sass {
                            l.a());
   }
 
-  Value* Eval::op_strings(Memory_Manager<AST_Node>& mem, enum Sass_OP op, Value& lhs, Value& rhs, bool compressed, int precision)
+  Value* Eval::op_strings(Memory_Manager& mem, enum Sass_OP op, Value& lhs, Value& rhs, bool compressed, int precision)
   {
     Expression::Concrete_Type ltype = lhs.concrete_type();
     Expression::Concrete_Type rtype = rhs.concrete_type();
@@ -1340,7 +1340,7 @@ namespace Sass {
     return SASS_MEMORY_NEW(mem, String_Constant, lhs.pstate(), (lstr) + sep + quote(rstr));
   }
 
-  Expression* cval_to_astnode(Memory_Manager<AST_Node>& mem, union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate)
+  Expression* cval_to_astnode(Memory_Manager& mem, union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate)
   {
     using std::strlen;
     using std::strcpy;

src/eval.hpp

@@ -89,18 +89,18 @@ namespace Sass {
     static bool eq(Expression*, Expression*);
     static bool lt(Expression*, Expression*);
     // -- arithmetic on the combinations that matter
-    static Value* op_numbers(Memory_Manager<AST_Node>&, enum Sass_OP, const Number&, const Number&, bool compressed = false, int precision = 5);
-    static Value* op_number_color(Memory_Manager<AST_Node>&, enum Sass_OP, const Number&, const Color&, bool compressed = false, int precision = 5);
-    static Value* op_color_number(Memory_Manager<AST_Node>&, enum Sass_OP, const Color&, const Number&, bool compressed = false, int precision = 5);
-    static Value* op_colors(Memory_Manager<AST_Node>&, enum Sass_OP, const Color&, const Color&, bool compressed = false, int precision = 5);
-    static Value* op_strings(Memory_Manager<AST_Node>&, enum Sass_OP, Value&, Value&, bool compressed = false, int precision = 5);
+    static Value* op_numbers(Memory_Manager&, enum Sass_OP, const Number&, const Number&, bool compressed = false, int precision = 5);
+    static Value* op_number_color(Memory_Manager&, enum Sass_OP, const Number&, const Color&, bool compressed = false, int precision = 5);
+    static Value* op_color_number(Memory_Manager&, enum Sass_OP, const Color&, const Number&, bool compressed = false, int precision = 5);
+    static Value* op_colors(Memory_Manager&, enum Sass_OP, const Color&, const Color&, bool compressed = false, int precision = 5);
+    static Value* op_strings(Memory_Manager&, enum Sass_OP, Value&, Value&, bool compressed = false, int precision = 5);
 
   private:
     std::string interpolation(Expression* s, bool into_quotes = false);
 
   };
 
-  Expression* cval_to_astnode(Memory_Manager<AST_Node>& mem, union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate = ParserState("[AST]"));
+  Expression* cval_to_astnode(Memory_Manager& mem, union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate = ParserState("[AST]"));
 
 }
 

src/memory_manager.cpp

@@ -3,18 +3,16 @@
 
 namespace Sass {
 
-  template <typename T>
-  Memory_Manager<T>::Memory_Manager(size_t size)
-  : nodes(std::vector<T*>())
+  Memory_Manager::Memory_Manager(size_t size)
+  : nodes(std::vector<Memory_Object*>())
   {
     size_t init = size;
     if (init < 8) init = 8;
     // reserve some space
     nodes.reserve(init);
   }
 
-  template <typename T>
-  Memory_Manager<T>::~Memory_Manager()
+  Memory_Manager::~Memory_Manager()
   {
     // release memory for all controlled nodes
     // avoid calling erase for every single node
@@ -25,64 +23,54 @@ namespace Sass {
     nodes.clear();
   }
 
-  template <typename T>
-  T* Memory_Manager<T>::add(T* np)
+  Memory_Object* Memory_Manager::add(Memory_Object* np)
   {
-    void* heap = (char*)np - 1;
-    *((char*)((void*)heap)) = 1;
+    // object has been initialized
+    // it can be "deleted" from now on
+    np->refcount = 1;
     return np;
   }
 
-  template <typename T>
-  bool Memory_Manager<T>::has(T* np)
+  bool Memory_Manager::has(Memory_Object* np)
   {
     // check if the pointer is controlled under our pool
     return find(nodes.begin(), nodes.end(), np) != nodes.end();
   }
 
-  template <typename T>
-  T* Memory_Manager<T>::allocate(size_t size)
+  Memory_Object* Memory_Manager::allocate(size_t size)
   {
-    // need additional memory for status header
-    void* heap = malloc(sizeof(char) + size);
-    // init internal status to zero
-    *(static_cast<char*>(heap)) = 0;
-    // the object lives on char further
-    void* object = static_cast<char*>(heap) + 1;
+    // allocate requested memory
+    void* heap = malloc(size);
+    // init internal refcount status to zero
+    (static_cast<Memory_Object*>(heap))->refcount = 0;
     // add the memory under our management
-    nodes.push_back(static_cast<T*>(object));
-    // cast object to its final type
-    return static_cast<T*>(object);
+    nodes.push_back(static_cast<Memory_Object*>(heap));
+    // cast object to its initial type
+    return static_cast<Memory_Object*>(heap);
   }
 
-  template <typename T>
-  void Memory_Manager<T>::deallocate(T* np)
+  void Memory_Manager::deallocate(Memory_Object* np)
   {
-    void* object = static_cast<void*>(np);
-    char* heap = static_cast<char*>(object) - 1;
-    if (heap[0]) np->~T();
-    free(heap);
+    // only call destructor if initialized
+    if (np->refcount) np->~Memory_Object();
+    // always free the memory
+    free(np);
   }
 
-  template <typename T>
-  void Memory_Manager<T>::remove(T* np)
+  void Memory_Manager::remove(Memory_Object* np)
   {
     // remove node from pool (no longer active)
     nodes.erase(find(nodes.begin(), nodes.end(), np));
     // you are now in control of the memory
   }
 
-  template <typename T>
-  void Memory_Manager<T>::destroy(T* np)
+  void Memory_Manager::destroy(Memory_Object* np)
   {
     // remove from pool
     remove(np);
     // release memory
     deallocate(np);
   }
 
-  // compile implementation for AST_Node
-  template class Memory_Manager<AST_Node>;
-
 }
 

src/memory_manager.hpp

@@ -4,27 +4,35 @@
 #include <vector>
 
 namespace Sass {
+
+  class Memory_Object {
+  friend class Memory_Manager;
+    long refcount;
+  public:
+    Memory_Object() { refcount = 0; };
+    virtual ~Memory_Object() {};
+  };
+
   /////////////////////////////////////////////////////////////////////////////
   // A class for tracking allocations of AST_Node objects. The intended usage
   // is something like: Some_Node* n = new (mem_mgr) Some_Node(...);
   // Then, at the end of the program, the memory manager will delete all of the
   // allocated nodes that have been passed to it.
   // In the future, this class may implement a custom allocator.
   /////////////////////////////////////////////////////////////////////////////
-  template <typename T>
   class Memory_Manager {
-    std::vector<T*> nodes;
+    std::vector<Memory_Object*> nodes;
 
   public:
     Memory_Manager(size_t size = 0);
     ~Memory_Manager();
 
-    bool has(T* np);
-    T* allocate(size_t size);
-    void deallocate(T* np);
-    void remove(T* np);
-    void destroy(T* np);
-    T* add(T* np);
+    bool has(Memory_Object* np);
+    Memory_Object* allocate(size_t size);
+    void deallocate(Memory_Object* np);
+    void remove(Memory_Object* np);
+    void destroy(Memory_Object* np);
+    Memory_Object* add(Memory_Object* np);
 
   };
 }

src/sass_values.cpp

@@ -348,7 +348,7 @@ extern "C" {
 
   union Sass_Value* ADDCALL sass_value_stringify (const union Sass_Value* v, bool compressed, int precision)
   {
-    Memory_Manager<AST_Node> mem;
+    Memory_Manager mem;
     Value* val = sass_value_to_ast_node(mem, v);
     std::string str(val->to_string(compressed, precision));
     return sass_make_qstring(str.c_str());
@@ -358,7 +358,7 @@ extern "C" {
   {
 
     Sass::Value* rv = 0;
-    Memory_Manager<AST_Node> mem;
+    Memory_Manager mem;
 
     try {