Merge pull request #2339 from mgreter/perf-refactor/loops-and-env

Performance improvements
sass · Feb 25, 2017 · e33894e · e33894e
2 parents 4c4af04 + ca70b9a
commit e33894e
Show file tree

Hide file tree

Showing 9 changed files with 293 additions and 102 deletions.
diff --git a/src/ast.hpp b/src/ast.hpp
@@ -169,6 +169,7 @@ namespace Sass {
       C_WARNING,
       C_ERROR,
       FUNCTION,
+      VARIABLE,
       NUM_TYPES
     };
     enum Simple_Type {
@@ -1512,10 +1513,10 @@ namespace Sass {
   public:
     Variable(ParserState pstate, std::string n)
     : PreValue(pstate), name_(n)
-    { }
+    { concrete_type(VARIABLE); }
     Variable(const Variable* ptr)
     : PreValue(ptr), name_(ptr->name_)
-    { }
+    { concrete_type(VARIABLE); }
 
     virtual bool operator==(const Expression& rhs) const
     {

diff --git a/src/ast_fwd_decl.hpp b/src/ast_fwd_decl.hpp
@@ -414,6 +414,9 @@ namespace Sass {
   typedef std::set<Compound_Selector_Obj, OrderNodes> CompoundSelectorSet;
   typedef std::unordered_set<Simple_Selector_Obj, HashNodes, CompareNodes> SimpleSelectorDict;
 
+  // only to switch implementations for testing
+  #define environment_map std::map
+
   // ###########################################################################
   // explicit type conversion functions
   // ###########################################################################

diff --git a/src/environment.cpp b/src/environment.cpp
@@ -6,17 +6,17 @@ namespace Sass {
 
   template <typename T>
   Environment<T>::Environment(bool is_shadow)
-  : local_frame_(std::map<std::string, T>()),
+  : local_frame_(environment_map<std::string, T>()),
     parent_(0), is_shadow_(false)
   { }
   template <typename T>
   Environment<T>::Environment(Environment<T>* env, bool is_shadow)
-  : local_frame_(std::map<std::string, T>()),
+  : local_frame_(environment_map<std::string, T>()),
     parent_(env), is_shadow_(is_shadow)
   { }
   template <typename T>
   Environment<T>::Environment(Environment<T>& env, bool is_shadow)
-  : local_frame_(std::map<std::string, T>()),
+  : local_frame_(environment_map<std::string, T>()),
     parent_(&env), is_shadow_(is_shadow)
   { }
 
@@ -45,20 +45,33 @@ namespace Sass {
   }
 
   template <typename T>
-  std::map<std::string, T>& Environment<T>::local_frame() {
+  environment_map<std::string, T>& Environment<T>::local_frame() {
     return local_frame_;
   }
 
   template <typename T>
   bool Environment<T>::has_local(const std::string& key) const
   { return local_frame_.find(key) != local_frame_.end(); }
 
+  template <typename T> EnvResult
+  Environment<T>::find_local(const std::string& key)
+  {
+    auto end = local_frame_.end();
+    auto it = local_frame_.find(key);
+    return EnvResult(it, it != end);
+  }
+
   template <typename T>
   T& Environment<T>::get_local(const std::string& key)
   { return local_frame_[key]; }
 
   template <typename T>
-  void Environment<T>::set_local(const std::string& key, T val)
+  void Environment<T>::set_local(const std::string& key, const T& val)
+  {
+    local_frame_[key] = val;
+  }
+  template <typename T>
+  void Environment<T>::set_local(const std::string& key, T&& val)
   {
     local_frame_[key] = val;
   }
@@ -86,7 +99,12 @@ namespace Sass {
   { return (*global_env())[key]; }
 
   template <typename T>
-  void Environment<T>::set_global(const std::string& key, T val)
+  void Environment<T>::set_global(const std::string& key, const T& val)
+  {
+    global_env()->local_frame_[key] = val;
+  }
+  template <typename T>
+  void Environment<T>::set_global(const std::string& key, T&& val)
   {
     global_env()->local_frame_[key] = val;
   }
@@ -126,13 +144,31 @@ namespace Sass {
   // either update already existing lexical value
   // or if flag is set, we create one if no lexical found
   template <typename T>
-  void Environment<T>::set_lexical(const std::string& key, T val)
+  void Environment<T>::set_lexical(const std::string& key, const T& val)
   {
-    auto cur = this;
+    Environment<T>* cur = this;
     bool shadow = false;
     while ((cur && cur->is_lexical()) || shadow) {
-      if (cur->has_local(key)) {
-        cur->set_local(key, val);
+      EnvResult rv(cur->find_local(key));
+      if (rv.found) {
+        rv.it->second = val;
+        return;
+      }
+      shadow = cur->is_shadow();
+      cur = cur->parent_;
+    }
+    set_local(key, val);
+  }
+  // this one moves the value
+  template <typename T>
+  void Environment<T>::set_lexical(const std::string& key, T&& val)
+  {
+    Environment<T>* cur = this;
+    bool shadow = false;
+    while ((cur && cur->is_lexical()) || shadow) {
+      EnvResult rv(cur->find_local(key));
+      if (rv.found) {
+        rv.it->second = val;
         return;
       }
       shadow = cur->is_shadow();
@@ -156,6 +192,20 @@ namespace Sass {
     return false;
   }
 
+  // look on the full stack for key
+  // include all scopes available
+  template <typename T> EnvResult
+  Environment<T>::find(const std::string& key)
+  {
+    auto cur = this;
+    while (true) {
+      EnvResult rv(cur->find_local(key));
+      if (rv.found) return rv;
+      cur = cur->parent_;
+      if (!cur) return rv;
+    }
+  };
+
   // use array access for getter and setter functions
   template <typename T>
   T& Environment<T>::operator[](const std::string& key)
@@ -177,7 +227,7 @@ namespace Sass {
     size_t indent = 0;
     if (parent_) indent = parent_->print(prefix) + 1;
     std::cerr << prefix << std::string(indent, ' ') << "== " << this << std::endl;
-    for (typename std::map<std::string, T>::iterator i = local_frame_.begin(); i != local_frame_.end(); ++i) {
+    for (typename environment_map<std::string, T>::iterator i = local_frame_.begin(); i != local_frame_.end(); ++i) {
       if (!ends_with(i->first, "[f]") && !ends_with(i->first, "[f]4") && !ends_with(i->first, "[f]2")) {
         std::cerr << prefix << std::string(indent, ' ') << i->first << " " << i->second;
         if (Value_Ptr val = Cast<Value>(i->second))

diff --git a/src/environment.hpp b/src/environment.hpp
@@ -2,17 +2,26 @@
 #define SASS_ENVIRONMENT_H
 
 #include <string>
-#include <map>
-
 #include "ast_fwd_decl.hpp"
 #include "ast_def_macros.hpp"
 
 namespace Sass {
 
+  typedef environment_map<std::string, AST_Node_Obj>::iterator EnvIter;
+
+  class EnvResult {
+    public:
+      EnvIter it;
+      bool found;
+    public:
+      EnvResult(EnvIter it, bool found)
+      : it(it), found(found) {}
+  };
+
   template <typename T>
   class Environment {
     // TODO: test with map
-    std::map<std::string, T> local_frame_;
+    environment_map<std::string, T> local_frame_;
     ADD_PROPERTY(Environment*, parent)
     ADD_PROPERTY(bool, is_shadow)
 
@@ -37,14 +46,17 @@ namespace Sass {
 
     // scope operates on the current frame
 
-    std::map<std::string, T>& local_frame();
+    environment_map<std::string, T>& local_frame();
 
     bool has_local(const std::string& key) const;
 
+    EnvResult find_local(const std::string& key);
+
     T& get_local(const std::string& key);
 
     // set variable on the current frame
-    void set_local(const std::string& key, T val);
+    void set_local(const std::string& key, const T& val);
+    void set_local(const std::string& key, T&& val);
 
     void del_local(const std::string& key);
 
@@ -60,7 +72,8 @@ namespace Sass {
     T& get_global(const std::string& key);
 
     // set a variable on the global frame
-    void set_global(const std::string& key, T val);
+    void set_global(const std::string& key, const T& val);
+    void set_global(const std::string& key, T&& val);
 
     void del_global(const std::string& key);
 
@@ -72,12 +85,17 @@ namespace Sass {
     // see if we have a lexical we could update
     // either update already existing lexical value
     // or we create a new one on the current frame
-    void set_lexical(const std::string& key, T val);
+    void set_lexical(const std::string& key, T&& val);
+    void set_lexical(const std::string& key, const T& val);
 
     // look on the full stack for key
     // include all scopes available
     bool has(const std::string& key) const;
 
+    // look on the full stack for key
+    // include all scopes available
+    EnvResult find(const std::string& key);
+
     // use array access for getter and setter functions
     T& operator[](const std::string& key);