Merge lp:~zorba-coders/zorba/objects-with-unordered-maps-and-const-char-star into lp:zorba

[Ghislain Fourny]

Changing JSON object implementation to unordered maps.

Changing JSON object implementation to unordered maps.

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/objects-with-unordered-maps-and-const-char-star-2012-09-13T15-44-40.281Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job objects-with-unordered-maps-and-const-char-star-2012-09-13T15-44-40.281Z is finished. The final status was:

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Pending.

[Matthias Brantner (matthias-brantner)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/objects-with-unordered-maps-and-const-char-star-2012-09-14T03-39-40.545Z/log.html

[Zorba Build Bot (zorba-buildbot)]

The attempt to merge lp:~zorba-coders/zorba/objects-with-unordered-maps-and-const-char-star into lp:zorba failed. Below is the output from the failed tests.

CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:274 (message):
  Validation queue job
  objects-with-unordered-maps-and-const-char-star-2012-09-14T03-39-40.545Z is
  finished. The final status was:

  3 tests did not succeed - changes not commited.

Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

[Ghislain Fourny (gislenius)]

Hi Matthias,

I merged some of my changes back and hope it fixes the failing tests.

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/objects-with-unordered-maps-and-const-char-star-2012-09-14T14-36-48.373Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job objects-with-unordered-maps-and-const-char-star-2012-09-14T14-36-48.373Z is finished. The final status was:

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Approve, 1 Pending.

[lp:~zorba-coders/zorba/objects-with-unordered-maps-and-const-char-star]

10996. By Matthias Brantner on 2012-09-14

merge

[Matthias Brantner (matthias-brantner)]

I benchmarked the following query in RelWithDebInfo:

for $i in 10 to 200
return {
  variable $o := {|
    for $j in 1 to $i
    return { xs:string($j) : "foo" }
  |};

  for $i in 1 to 20
  return {
    $o(string($i));
    $o(string($i - 5));
    $o(string($i - 8));
  }
}

std::unordered_map (this branch)

Number of executions = 5
Average Execution Time : 192.889 (user: 192.556) milliseconds
Average Total Time : 194.554 (user: 194.217) milliseconds
real 0m1.105s
user 0m1.092s
sys 0m0.008s

HashMap (trunk)

Number of executions = 5
Average Execution Time : 271.695 (user: 271.224) milliseconds
Average Total Time : 273.574 (user: 273.099) milliseconds
real 0m1.501s
user 0m1.484s
sys 0m0.016s

review: Approve

[Ghislain Fourny (gislenius)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/objects-with-unordered-maps-and-const-char-star-2012-09-24T15-46-14.382Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job objects-with-unordered-maps-and-const-char-star-2012-09-24T15-46-14.382Z is finished. The final status was:

All tests succeeded!

=== modified file 'src/store/naive/json_items.cpp'
--- src/store/naive/json_items.cpp	2012-09-14 15:32:12 +0000
+++ src/store/naive/json_items.cpp	2012-09-14 16:01:19 +0000
@@ -292,9 +292,9 @@
     bool accumulate)
 {
   ASSERT_INVARIANT();
-  zstring lName = aName->getStringValue();
+  const char* lName = aName->getStringValue().c_str();
 
-  Keys::iterator ite = theKeys.find(lName.c_str());
+  Keys::iterator ite = theKeys.find(lName);
 
   if (ite == theKeys.end())
   {
@@ -308,7 +308,7 @@
     }
     
     csize lPosition = thePairs.size();
-    theKeys.insert(lName.c_str(), lPosition);
+    theKeys.insert(std::make_pair(lName, lPosition));
     thePairs.push_back(std::make_pair(aName.getp(), lValue));
     aName->addReference();
     lValue->addReference();
@@ -318,7 +318,7 @@
   }
   else if (accumulate)
   {
-    csize lPosition = ite.getValue();
+    csize lPosition = ite->second;
 
     assert(thePairs[lPosition].first->getStringValue() == lName);
 
@@ -359,15 +359,16 @@
 {
   ASSERT_INVARIANT();
 
-  zstring lName = aName->getStringValue();
-  csize lPosition = 0;
+  const char* lName = aName->getStringValue().c_str();
   store::Item_t lValue;
 
-  if (!theKeys.get(lName.c_str(), lPosition))
+  Keys::iterator lIter = theKeys.find(lName);
+  if (lIter == theKeys.end())
   {
     ASSERT_INVARIANT();
     return 0;
   }
+  csize lPosition = lIter->second;
   
   store::Item* lKey;
 
@@ -385,7 +386,7 @@
   lValue->removeReference();
 
   thePairs.erase(thePairs.begin() + lPosition);
-  theKeys.erase(lName.c_str());
+  theKeys.erase(lIter);
 
   if (lPosition < thePairs.size())
   {
@@ -393,10 +394,10 @@
     Keys::iterator lKeysEnd = theKeys.end();
     for (; lKeysIte != lKeysEnd; ++lKeysIte)
     {
-      csize lPos = lKeysIte.getValue();
+      csize lPos = lKeysIte->second;
       if (lPos > lPosition)
       {
-        lKeysIte.setValue(lPos - 1);
+        lKeysIte->second = lPos - 1;
       }
     }
   }
@@ -414,14 +415,15 @@
     const store::Item_t& aValue)
 {
   ASSERT_INVARIANT();
-  zstring lName = aName->getStringValue();
-  csize lPosition = 0;
+  const char* lName = aName->getStringValue().c_str();
 
-  if (!theKeys.get(lName.c_str(), lPosition))
+  Keys::const_iterator lIter = theKeys.find(lName);
+  if (lIter == theKeys.end())
   {
     ASSERT_INVARIANT();
-    return NULL;
+    return 0;
   }
+  csize lPosition = lIter->second;
 
   assert(thePairs[lPosition].first->getStringValue() == lName);
 
@@ -461,16 +463,17 @@
     const store::Item_t& aNewName)
 {
   ASSERT_INVARIANT();
-  zstring lName = aName->getStringValue();
-  zstring lNewName = aNewName->getStringValue();
+  const char* lName = aName->getStringValue().c_str();
+  const char* lNewName = aNewName->getStringValue().c_str();
 
-  if (theKeys.exists(lNewName.c_str()))
+  Keys::const_iterator lIter = theKeys.find(lNewName);
+  if (lIter != theKeys.end())
   {
     ASSERT_INVARIANT();
     return false;
   }
 
-  Keys::iterator ite = theKeys.find(lName.c_str());
+  Keys::iterator ite = theKeys.find(lName);
 
   if (ite == theKeys.end())
   {
@@ -478,14 +481,14 @@
     return false;
   }
 
-  csize lPosition = ite.getValue();
+  csize lPosition = ite->second;
   assert(thePairs[lPosition].first->getStringValue() == lName);
   
   thePairs[lPosition].first->removeReference();
   aNewName->addReference();
   thePairs[lPosition].first = aNewName.getp();
   theKeys.erase(ite);
-  theKeys.insert(lNewName.c_str(), lPosition);
+  theKeys.insert(std::make_pair(lNewName, lPosition));
 
   ASSERT_INVARIANT();
   return true;
@@ -560,13 +563,15 @@
 store::Item_t SimpleJSONObject::getObjectValue(const store::Item_t& aKey) const
 {
   ASSERT_INVARIANT();
-  zstring lName = aKey->getStringValue();
-
-  csize lPosition = 0;
-  if (!theKeys.get(lName.c_str(), lPosition))
+  const char* lName = aKey->getStringValue().c_str();
+
+  Keys::const_iterator lIter = theKeys.find(lName);
+
+  if (lIter == theKeys.end())
   {
     return NULL;
   }
+  csize lPosition = lIter->second;
 
   assert(thePairs[lPosition].first->equals(aKey));
   return thePairs[lPosition].second;
@@ -593,15 +598,15 @@
   JSONItem::assertInvariant();
   assert(theKeys.size() == thePairs.size());
 
-  for(Keys::iterator lIter = theKeys.begin();
+  for(Keys::const_iterator lIter = theKeys.begin();
       lIter != theKeys.end();
       ++lIter)
   {
-    csize lPosition = lIter.getValue();
+    csize lPosition = lIter->second;
     assert(lPosition < thePairs.size());
     assert(thePairs[lPosition].first != NULL);
     assert(thePairs[lPosition].first->isAtomic());
-    assert(thePairs[lPosition].first->getStringValue() == lIter.getKey());
+    assert(thePairs[lPosition].first->getStringValue() == lIter->first);
     assert(thePairs[lPosition].second != NULL);
   }
 }

=== modified file 'src/store/naive/json_items.h'
--- src/store/naive/json_items.h	2012-09-14 15:32:12 +0000
+++ src/store/naive/json_items.h	2012-09-14 16:01:19 +0000
@@ -17,11 +17,10 @@
 #ifndef ZORBA_STORE_JSON_ITEMS_H
 #define ZORBA_STORE_JSON_ITEMS_H
 
-#include <set>
 #include <vector>
 
 #include <zorba/config.h>
-#include <zorbautils/hashmap_cstring.h>
+#include "util/unordered_map.h"
 
 #include "store/api/item_handle.h"
 #include "store/api/iterator.h"
@@ -241,8 +240,36 @@
 class SimpleJSONObject : public JSONObject
 {
 protected:
-  CSTRING_HASH_MAP(csize, Keys);
-
+  class ConstCharStarHash
+  {
+  public:
+    typedef size_t result_type;
+    size_t operator()(const char* a) const
+    {
+      size_t hash = 5381;
+      int c;
+
+      while ((c = *a++))
+        hash = ((hash << 5) + hash) + c;
+
+      return hash;
+    }
+  };
+
+  class ConstCharStarComparator
+  {
+  public:
+    bool operator()(const char* a, const char* b) const
+    {
+      return strcmp(a, b) == 0;
+    }
+  };
+
+  typedef std::unordered_map<
+    const char*,
+    csize,
+    ConstCharStarHash,
+    ConstCharStarComparator> Keys;
   typedef std::vector<std::pair<store::Item*, store::Item*> > Pairs;
 
   class KeyIterator : public store::Iterator
@@ -271,10 +298,7 @@
 
 public:
   SimpleJSONObject()
-    :
-    theKeys(64, false)
-  {
-  }
+  {}
 
   virtual ~SimpleJSONObject();