Changes to hashing that solve the xinetd boolean benchmark in 14s (from ~25min)....
authorDejan Jovanović <dejan.jovanovic@gmail.com>
Fri, 12 Feb 2010 01:07:22 +0000 (01:07 +0000)
committerDejan Jovanović <dejan.jovanovic@gmail.com>
Fri, 12 Feb 2010 01:07:22 +0000 (01:07 +0000)
src/expr/expr.cpp
src/expr/expr.h
src/expr/node.cpp
src/expr/node.h
src/expr/node_builder.h
src/expr/node_manager.cpp
src/expr/node_manager.h
src/expr/node_value.cpp
src/expr/node_value.h
src/parser/smt/smt_parser.g

index 283467fa8da8a7ed160432f30395e613a2af6c2b..0170296be3d88aa93e2d802b0556b7303e9f302d 100644 (file)
@@ -77,7 +77,7 @@ bool Expr::operator<(const Expr& e) const {
   return *d_node < *e.d_node;
 }
 
-uint64_t Expr::hash() const {
+size_t Expr::hash() const {
   Assert(d_node != NULL, "Unexpected NULL expression pointer!");
   return (d_node->isNull());
 }
index 6e4a43d21f8c290ce473cb3f097728ef9101711f..0943c13e4261fbab14aef97b2616db2122992879 100644 (file)
@@ -92,7 +92,7 @@ public:
    * Returns the hash value of the expression. Equal expression will have the
    * same hash value.
    */
-  uint64_t hash() const;
+  size_t hash() const;
 
   /**
    * Returns the kind of the expression (AND, PLUS ...).
index 6f1c525cac9c17a440d0180f5f006797e47f7325..f3115b17e9e47b423d682fd6b2e05befabd83828 100644 (file)
@@ -96,11 +96,6 @@ Node& Node::operator=(const Node& e) {
   return *this;
 }
 
-uint64_t Node::hash() const {
-  Assert(d_ev != NULL, "Expecting a non-NULL expression value!");
-  return d_ev->hash();
-}
-
 Node Node::eqExpr(const Node& right) const {
   return NodeManager::currentNM()->mkNode(EQUAL, *this, right);
 }
index 3b0cb8abb15ffdd7026efd75cf6015696483aca0..39eb3bcd7b54a5e21e94c3eb3bd70c4d35779e80 100644 (file)
@@ -119,7 +119,7 @@ public:
 
   Node& operator=(const Node&);
 
-  uint64_t hash() const;
+  size_t hash() const { return d_ev->getId(); }
   uint64_t getId() const { return d_ev->getId(); }
 
   Node eqExpr(const Node& right) const;
index 765a2f49384819fc39849e74702d7c7deb214fb4..ce56fc08c5217dfd1cee1e3859ecc0ca878abbec 100644 (file)
@@ -192,7 +192,6 @@ public:
     Debug("prop") << "append: " << this << " " << n << "[" << n.d_ev << "]" << std::endl;
     allocateEvIfNecessaryForAppend();
     NodeValue* ev = n.d_ev;
-    d_hash = NodeValue::computeHash(d_hash, ev);
     ev->inc();
     d_ev->d_children[d_ev->d_nchildren++] = ev;
     return *this;
@@ -587,7 +586,9 @@ NodeBuilder<nchild_thresh>::operator Node() {// not const
   // was malloc'ed or not
 
   if(EXPECT_FALSE( evIsAllocated() )) {
-    NodeValue *ev = d_nm->lookupNoInsert(d_hash, d_ev);
+    // Lookup the expression value in the pool we already have (with insert)
+    NodeValue* ev = d_nm->poolLookup(d_ev);
+    // If something else is there, we reuse it
     if(ev != NULL) {
       // expression already exists in node manager
       dealloc();
@@ -595,20 +596,19 @@ NodeBuilder<nchild_thresh>::operator Node() {// not const
       Debug("prop") << "result: " << Node(ev) << std::endl;
       return Node(ev);
     }
-
-    // otherwise create the canonical expression value for this node
+    // Otherwise crop and set the expression value to the allocate one
     crop();
     ev = d_ev;
     d_ev = NULL;
-    // this inserts into the NodeManager;
-    // return the result of lookup() in case another thread beat us to it
     d_used = true;
-    Node n = d_nm->lookup(d_hash, ev);
+    d_nm->poolInsert(ev);
+    Node n(ev);
     Debug("prop") << "result: " << n << std::endl;
     return n;
   }
 
-  NodeValue *ev = d_nm->lookupNoInsert(d_hash, &d_inlineEv);
+  // Lookup the expression value in the pool we already have
+  NodeValue* ev = d_nm->poolLookup(&d_inlineEv);
   if(ev != NULL) {
     // expression already exists in node manager
     d_used = true;
@@ -630,12 +630,9 @@ NodeBuilder<nchild_thresh>::operator Node() {// not const
   d_used = true;
   d_ev = NULL;
 
-  // this inserts into the NodeManager;
-  // return the result of lookup() in case another thread beat us to it
-  if(ev->getNumChildren()) {
-    Debug("prop") << "ev first child: " << *ev->ev_begin() << std::endl;
-  }
-  Node n = d_nm->lookup(d_hash, ev);
+  // Make the new expression
+  d_nm->poolInsert(ev);
+  Node n(ev);
   Debug("prop") << "result: " << n << std::endl;
   return n;
 }
index e9fdc69a80b926a386644714f63aee1faa957b4c..3561f211997168e3982f826e111e773d56ff5851 100644 (file)
@@ -22,88 +22,25 @@ namespace CVC4 {
 
 __thread NodeManager* NodeManager::s_current = 0;
 
-Node NodeManager::lookup(uint64_t hash, NodeValue* ev) {
-  Assert(this != NULL, "Whoops, we have a bad expression manager!");
-  Assert(ev != NULL, "lookup() expects a non-NULL NodeValue!");
-
-  hash_t::iterator i = d_hash.find(hash);
-  if(i == d_hash.end()) {
-    // insert
-    std::vector<Node> v;
-    Node e(ev);
-    v.push_back(e);
-    d_hash.insert(std::make_pair(hash, v));
-    return e;
-  } else {
-    for(std::vector<Node>::iterator j = i->second.begin(); j != i->second.end(); ++j) {
-      if(ev->getKind() != j->getKind()) {
-        continue;
-      }
-
-      if(ev->getNumChildren() != j->getNumChildren()) {
-        continue;
-      }
-
-      NodeValue::const_ev_iterator c1 = ev->ev_begin();
-      NodeValue::ev_iterator c2 = j->d_ev->ev_begin();
-      for(; c1 != ev->ev_end() && c2 != j->d_ev->ev_end(); ++c1, ++c2) {
-        if(*c1 != *c2) {
-          break;
-        }
-      }
-
-      if(c1 != ev->ev_end() || c2 != j->d_ev->ev_end()) {
-        continue;
-      }
-
-      return *j;
-    }
-
-    // didn't find it, insert
-    Node e(ev);
-    i->second.push_back(e);
-    return e;
-  }
+NodeManager::NodeManager() {
+  poolInsert(&NodeValue::s_null);
 }
 
-NodeValue* NodeManager::lookupNoInsert(uint64_t hash, NodeValue* ev) {
-  Assert(this != NULL, "Whoops, we have a bad expression manager!");
-  Assert(ev != NULL, "lookupNoInsert() expects a non-NULL NodeValue!");
-
-  hash_t::iterator i = d_hash.find(hash);
-  if(i == d_hash.end()) {
+NodeValue* NodeManager::poolLookup(NodeValue* nv) const {
+  NodeValueSet::const_iterator find = d_nodeValueSet.find(nv);
+  if (find == d_nodeValueSet.end()) {
     return NULL;
   } else {
-    for(std::vector<Node>::iterator j = i->second.begin(); j != i->second.end(); ++j) {
-      if(ev->getKind() != j->getKind()) {
-        continue;
-      }
-
-      if(ev->getNumChildren() != j->getNumChildren()) {
-        continue;
-      }
-
-      NodeValue::const_ev_iterator c1 = ev->ev_begin();
-      NodeValue::ev_iterator c2 = j->d_ev->ev_begin();
-      for(; c1 != ev->ev_end() && c2 != j->d_ev->ev_end(); ++c1, ++c2) {
-        Debug("expr") << "comparing " << c1 << " and " << c2 << std::endl;
-        if(*c1 != *c2) {
-          break;
-        }
-      }
-
-      if(c1 != ev->ev_end() || c2 != j->d_ev->ev_end()) {
-        continue;
-      }
-
-      return j->d_ev;
-    }
-
-    // didn't find it, don't insert
-    return 0;
+    return *find;
   }
 }
 
+void NodeManager::poolInsert(NodeValue* nv) {
+  Assert(d_nodeValueSet.find(nv) == d_nodeValueSet.end(), "NodeValue already in"
+         "the pool!");
+  d_nodeValueSet.insert(nv);
+}
+
 // general expression-builders
 
 Node NodeManager::mkNode(Kind kind) {
index 090398ce88f25c4ee65eebabfd936fae39bcea8d..0abd8613015ecb54c2ce69dceaeb219cfd42815e 100644 (file)
 #define __CVC4__NODE_MANAGER_H
 
 #include <vector>
-#include <map>
+#include <ext/hash_set>
 
 #include "node.h"
 #include "kind.h"
 
+namespace __gnu_cxx {
+template<>
+  struct hash<CVC4::NodeValue*> {
+    size_t operator()(const CVC4::NodeValue* nv) const {
+      return (size_t)nv->hash();
+    }
+  };
+} /* __gnu_cxx namespace */
+
+
 namespace CVC4 {
 
 class NodeManager {
@@ -29,16 +39,18 @@ class NodeManager {
 
   template <unsigned> friend class CVC4::NodeBuilder;
 
-  typedef std::map<uint64_t, std::vector<Node> > hash_t;
-  hash_t d_hash;
+  typedef __gnu_cxx::hash_set<NodeValue*, __gnu_cxx::hash<NodeValue*>, NodeValue::NodeValueEq> NodeValueSet;
+  NodeValueSet d_nodeValueSet;
 
-  Node lookup(uint64_t hash, const Node& e) { return lookup(hash, e.d_ev); }
-  Node lookup(uint64_t hash, NodeValue* e);
-  NodeValue* lookupNoInsert(uint64_t hash, NodeValue* e);
+  NodeValue* poolLookup(NodeValue* nv) const;
+  void poolInsert(NodeValue* nv);
 
   friend class NodeManagerScope;
 
 public:
+
+  NodeManager();
+
   static NodeManager* currentNM() { return s_current; }
 
   // general expression-builders
index 575ab7d37a7513eb5c744de330bae8934a70dd68..f8bf33b5c04ddf3a238745d0ba250dc0f750a4b7 100644 (file)
@@ -40,10 +40,6 @@ NodeValue::~NodeValue() {
   }
 }
 
-uint64_t NodeValue::hash() const {
-  return computeHash(d_kind, ev_begin(), ev_end());
-}
-
 void NodeValue::inc() {
   // FIXME multithreading
   if(EXPECT_TRUE( d_rc < MAX_RC )) {
index bb224e3b1398ccb7f123703c6e7a4d864e13648f..d86822b8dc81f19904443b61f8e569e1373d8a9f 100644 (file)
@@ -88,29 +88,6 @@ class NodeValue {
   /** Destructor decrements the ref counts of its children */
   ~NodeValue();
 
-  /**
-   * Computes the hash over the given iterator span of children, and the
-   * root hash. The iterator should be either over a range of Node or pointers
-   * to NodeValue.
-   * @param hash the initial value for the hash
-   * @param begin the begining of the range
-   * @param end the end of the range
-   * @return the hash value
-   */
-  template<typename const_iterator_type>
-  static uint64_t computeHash(uint64_t hash, const_iterator_type begin,
-                              const_iterator_type end) {
-    for(const_iterator_type i = begin; i != end; ++i) {
-      hash = computeHash(hash, *i);
-    }
-    return hash;
-  }
-
-  static uint64_t computeHash(uint64_t hash, const NodeValue* ev) {
-    return
-      ( (hash << 3) | ((hash & 0xE000000000000000ull) >> 61) ) ^ ev->getId();
-  }
-
   typedef NodeValue** ev_iterator;
   typedef NodeValue const* const* const_ev_iterator;
 
@@ -153,12 +130,8 @@ class NodeValue {
   typedef node_iterator const_node_iterator;
 
 public:
-  /** Hash this expression.
-   *  @return the hash value of this expression. */
-  uint64_t hash() const;
 
   // Iterator support
-
   typedef node_iterator iterator;
   typedef node_iterator const_iterator;
 
@@ -168,6 +141,44 @@ public:
   const_iterator begin() const;
   const_iterator end() const;
 
+  /**
+   * Hash this expression.
+   * @return the hash value of this expression.
+   */
+  size_t hash() const {
+    size_t hash = d_kind;
+    const_ev_iterator i = ev_begin();
+    const_ev_iterator i_end = ev_end();
+    while (i != i_end) {
+      hash ^= (*i)->d_id + 0x9e3779b9 + (hash << 6) + (hash >> 2);
+      ++ i;
+    }
+    return hash;
+  }
+
+  inline bool compareTo(const NodeValue* nv) const {
+    if(d_kind != nv->d_kind)
+      return false;
+    if(d_nchildren != nv->d_nchildren)
+      return false;
+    const_ev_iterator i = ev_begin();
+    const_ev_iterator j = nv->ev_begin();
+    const_ev_iterator i_end = ev_end();
+    while(i != i_end) {
+      if ((*i) != (*j)) return false;
+      ++i; ++j;
+    }
+    return true;
+  }
+
+  // Comparison predicate
+  struct NodeValueEq {
+    bool operator()(const NodeValue* nv1, const NodeValue* nv2) const {
+      return nv1->compareTo(nv2);
+    }
+  };
+
+
   unsigned getId() const { return d_id; }
   Kind getKind() const { return (Kind) d_kind; }
   unsigned getNumChildren() const { return d_nchildren; }
index dbc9e1a219567d57a547ce79f5226cc145224374..4ecac041897eb4e65c3763b852fb932ea3d09762 100644 (file)
@@ -117,7 +117,7 @@ benchAttribute returns [Command* smt_command = 0]
 annotatedFormula returns [CVC4::Expr formula]
 {
   Debug("parser") << "annotated formula: " << LT(1)->getText() << endl;
-  Kind kind;
+  Kind kind = UNDEFINED_KIND;
   vector<Expr> args; 
 }
   : /* a built-in operator application */