Merge lp:~matthias-brantner/zorba/bug-fixing into lp:zorba

[Matthias Brantner]

Optimization. Lazily create the external function parameter hashmap. This saves a lot of time every time the dynamic context is copied, i.e. on every function invocation.

Optimization. Lazily create the external function parameter hashmap. This saves a lot of time every time the dynamic context is copied, i.e. on every function invocation.

[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/bug-fixing-2011-10-18T22-36-08.666Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job bug-fixing-2011-10-18T22-36-08.666Z 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. Got: 1 Approve, 1 Pending.

[Markos Zaharioudakis (markos-za)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug-fixing-2011-10-18T23-00-00.712Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job bug-fixing-2011-10-18T23-00-00.712Z is finished. The final status was:

All tests succeeded!

[lp:~matthias-brantner/zorba/bug-fixing]

10520. By Matthias Brantner on 2011-10-18

Optimization. Lazily create the external function parameter hashmap. This saves a lot of time every time the dynamic context is copied, i.e. on every function invocation. Approved: Markos Zaharioudakis, Matthias Brantner

=== modified file 'src/compiler/translator/translator.cpp'
--- src/compiler/translator/translator.cpp	2011-10-03 09:18:49 +0000
+++ src/compiler/translator/translator.cpp	2011-10-18 22:22:26 +0000
@@ -101,6 +101,7 @@
 #include "util/tracer.h"
 #include "util/utf8_util.h"
 #include "util/xml_util.h"
+#include "util/hashmap.h"
 
 
 #define NODE_SORT_OPT

=== modified file 'src/context/dynamic_context.cpp'
--- src/context/dynamic_context.cpp	2011-10-03 09:18:49 +0000
+++ src/context/dynamic_context.cpp	2011-10-18 22:22:26 +0000
@@ -120,6 +120,7 @@
 dynamic_context::dynamic_context(dynamic_context* parent)
   :
   theParent(NULL),
+  keymap(NULL),
   theAvailableIndices(NULL),
   theDocLoadingUserTime(0.0),
   theDocLoadingTime(0)
@@ -144,13 +145,20 @@
 ********************************************************************************/
 dynamic_context::~dynamic_context()
 {
-  for (uint32_t i = 0; i < keymap.size(); ++i)
+  if (keymap)
   {
-    dctx_value_t lValue = keymap.getentryVal(i);
-    if (lValue.type == dctx_value_t::ext_func_param_typed && lValue.func_param)
+    for (ValueMap::iterator lIter = keymap->begin();
+         lIter != keymap->end();
+         ++lIter)
     {
-      static_cast<ExternalFunctionParameter*>(lValue.func_param)->destroy();
+      dctx_value_t lValue = lIter.getValue();
+      if (lValue.type == dctx_value_t::ext_func_param_typed &&
+          lValue.func_param)
+      {
+        static_cast<ExternalFunctionParameter*>(lValue.func_param)->destroy();
+      }
     }
+    delete keymap;
   }
 
   if (theAvailableIndices)
@@ -572,7 +580,20 @@
   val.type = dynamic_context::dctx_value_t::ext_func_param;
   val.func_param = aValue;
 
-  return keymap.put ( aName, val);
+  if (!keymap)
+  {
+    keymap = new ValueMap(8, false);
+  }
+
+  if (!keymap->insert(aName, val))
+  {
+    keymap->update(aName, val);
+    return false;
+  }
+  else
+  {
+    return true;
+  }
 }
 
 
@@ -583,11 +604,24 @@
   const std::string& aName,
   void*& aValue) const
 {
+  if (!keymap)
+  {
+    if (theParent)
+    {
+      return theParent->getExternalFunctionParam(aName, aValue);
+    }
+    else
+    {
+      return false;
+    }
+  }
+
   dctx_value_t val;
   val.type = dynamic_context::dctx_value_t::no_val;
   val.func_param = 0;
 
-  if ( !keymap.get(aName, val) ) 
+  ValueMap::iterator lIter = keymap->find(aName);
+  if ( lIter == keymap->end() )
   {
     if (theParent)
       return theParent->getExternalFunctionParam(aName, aValue);
@@ -595,6 +629,8 @@
       return false;
   }
 
+  val = lIter.getValue();
+
   if (val.type == dynamic_context::dctx_value_t::ext_func_param)
   {
     aValue = val.func_param;
@@ -615,6 +651,11 @@
    const std::string& aName,
    ExternalFunctionParameter* aValue)
 {
+  if (!keymap)
+  {
+    keymap = new ValueMap(8, false);
+  }
+
   dctx_value_t val;
   val.type = dynamic_context::dctx_value_t::ext_func_param_typed;
   val.func_param = aValue;
@@ -624,8 +665,15 @@
   {
     // destroy the object if it's already contained in the map
     lValue->destroy();
-  }
-  return keymap.put ( aName, val);
+    keymap->erase(aName);
+    keymap->insert(aName, val);
+    return false;
+  }
+  else
+  {
+    keymap->insert(aName, val);
+    return true;
+  }
 }
 
 
@@ -635,11 +683,24 @@
 ExternalFunctionParameter*
 dynamic_context::getExternalFunctionParameter(const std::string& aName) const
 {
+  if (!keymap)
+  {
+    if (theParent)
+    {
+      return theParent->getExternalFunctionParameter(aName);
+    }
+    else
+    {
+      return 0;
+    }
+  }
+
   dctx_value_t val;
   val.type = dynamic_context::dctx_value_t::no_val;
   val.func_param = 0;
 
-  if ( !keymap.get(aName, val) ) 
+  ValueMap::iterator lIter = keymap->find(aName);
+  if (lIter == keymap->end())
   {
     if (theParent)
       return theParent->getExternalFunctionParameter(aName);
@@ -647,28 +708,14 @@
       return 0;
   }
 
+  val = lIter.getValue();
+
   ExternalFunctionParameter* lRes = 
   static_cast<ExternalFunctionParameter*>(val.func_param);
 
   return lRes;
 }
 
-/*
-std::vector<zstring>* dynamic_context::get_all_keymap_keys() const
-{
-  std::auto_ptr<std::vector<zstring> > keys;
-  if (theParent != NULL)
-    keys.reset(theParent->get_all_keymap_keys());
-  else
-    keys.reset(new std::vector<zstring>);
-
-  for (unsigned int i=0; i<keymap.size(); i++)
-    keys->push_back(keymap.getentryKey(i));
-
-  return keys.release();
-}
-*/
-
 
 } // namespace zorba
 /* vim:set et sw=2 ts=2: */

=== modified file 'src/context/dynamic_context.h'
--- src/context/dynamic_context.h	2011-10-03 09:18:49 +0000
+++ src/context/dynamic_context.h	2011-10-18 22:22:26 +0000
@@ -17,7 +17,8 @@
 #ifndef ZORBA_DYNAMIC_CONTEXT_H
 #define ZORBA_DYNAMIC_CONTEXT_H
 
-#include "util/hashmap.h"
+#include <zorba/external_function_parameter.h>
+#include "zorbautils/hashmap_zstring_nonserializable.h"
 
 #include "common/shared_types.h"
 
@@ -94,8 +95,7 @@
     ~VarValue(); 
   };
 
-  // QQQ zstring?
-  typedef hashmap<std::string, dctx_value_t> ValueMap;
+  typedef HashMapZString<dctx_value_t> ValueMap;
 
   typedef ItemPointerHashMap<store::Index_t> IndexMap;
 
@@ -109,7 +109,7 @@
 
   std::vector<VarValue>        theVarValues;
 
-  ValueMap                     keymap;
+  ValueMap                   * keymap;
 
   IndexMap                   * theAvailableIndices;
 
@@ -202,12 +202,19 @@
   ExternalFunctionParameter* getExternalFunctionParameter(
       const std::string& aName) const;
 
-  //std::vector<zstring>* get_all_keymap_keys() const;
-
 protected:
   bool lookup_once(const std::string& key, dctx_value_t& val) const
   {
-    return keymap.get(key, val);
+    if (keymap)
+    {
+      ValueMap::iterator lIter = keymap->find(key);
+      if (lIter != keymap->end())
+      {
+        val = lIter.getValue();
+        return true;
+      }
+    }
+    return false;
   }
 
   bool context_value(const std::string& key, dctx_value_t& val) const
@@ -223,7 +230,7 @@
   {
     if (lookup_once (key, val))
     {
-      if (map != NULL) *map = &keymap;
+      if (map != NULL) *map = keymap;
       return true;
     }
     return theParent == NULL ? false : theParent->context_value(key, val, map);

=== modified file 'src/zorbaserialization/zorba_class_versions.cpp'
--- src/zorbaserialization/zorba_class_versions.cpp	2011-06-14 17:26:33 +0000
+++ src/zorbaserialization/zorba_class_versions.cpp	2011-10-18 22:22:26 +0000
@@ -130,22 +130,22 @@
 
 
 // HashMapZString
-SERIALIZABLE_CLASS_VERSIONS(HashMapZStringCmp)
-END_SERIALIZABLE_CLASS_VERSIONS(HashMapZStringCmp)
+SERIALIZABLE_CLASS_VERSIONS(serializable_HashMapZStringCmp)
+END_SERIALIZABLE_CLASS_VERSIONS(serializable_HashMapZStringCmp)
 
 SERIALIZABLE_TEMPLATE_VERSIONS(serializable_HashMapZString)
 END_SERIALIZABLE_TEMPLATE_VERSIONS(serializable_HashMapZString)
 
 SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS2(serializable_HashEntry, serializable_HashEntry<zstring, static_context::ctx_module_t>, 11)
 
-SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS3(serializable_HashMap, serializable_HashMap<zstring, static_context::ctx_module_t, HashMapZStringCmp>, 11)
+SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS3(serializable_HashMap, serializable_HashMap<zstring, static_context::ctx_module_t, serializable_HashMapZStringCmp>, 11)
 
 SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS(serializable_HashMapZString, serializable_HashMapZString<static_context::ctx_module_t>, 1)
 
 
 SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS2(serializable_HashEntry, serializable_HashEntry<zstring, zstring>, 12)
 
-SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS3(serializable_HashMap, serializable_HashMap<zstring, zstring, HashMapZStringCmp>,12)
+SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS3(serializable_HashMap, serializable_HashMap<zstring, zstring, serializable_HashMapZStringCmp>,12)
 
 SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS(serializable_HashMapZString, serializable_HashMapZString<zstring>, 2)
 
@@ -155,7 +155,7 @@
 
 SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS(serializable_HashMapZString, serializable_HashMapZString<xqtref_t>, 3)
 
-SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS3(serializable_HashMap, serializable_HashMap<zstring, xqtref_t, HashMapZStringCmp>,13)
+SERIALIZABLE_TEMPLATE_INSTANCE_VERSIONS3(serializable_HashMap, serializable_HashMap<zstring, xqtref_t, serializable_HashMapZStringCmp>,13)
 
 
 

=== modified file 'src/zorbautils/hashmap_zstring.h'
--- src/zorbautils/hashmap_zstring.h	2011-06-14 17:26:33 +0000
+++ src/zorbautils/hashmap_zstring.h	2011-10-18 22:22:26 +0000
@@ -33,7 +33,7 @@
   Class to privide the equality and hash functions for the HashMapZString
   class defined below.
 *******************************************************************************/
-class HashMapZStringCmp : public ::zorba::serialization::SerializeBaseClass
+class serializable_HashMapZStringCmp : public ::zorba::serialization::SerializeBaseClass
 {
 public:
   static uint32_t hash(const zstring& str)
@@ -47,9 +47,9 @@
   }
 
 public:
-  SERIALIZABLE_CLASS(HashMapZStringCmp);
+  SERIALIZABLE_CLASS(serializable_HashMapZStringCmp);
 
-  HashMapZStringCmp(::zorba::serialization::Archiver& ar)
+  serializable_HashMapZStringCmp(::zorba::serialization::Archiver& ar)
   {
   }
 
@@ -57,7 +57,7 @@
   {
   }
 
-  HashMapZStringCmp()
+  serializable_HashMapZStringCmp()
   {
   }
 };
@@ -70,27 +70,27 @@
 template<class V>
 class serializable_HashMapZString : public serializable_HashMap<zstring,
                                                                 V,
-                                                                HashMapZStringCmp>
+                                                                serializable_HashMapZStringCmp>
 {
 public:
   SERIALIZABLE_TEMPLATE_CLASS(serializable_HashMapZString)
 
   serializable_HashMapZString(::zorba::serialization::Archiver& ar)
     :
-    serializable_HashMap<zstring, V, HashMapZStringCmp>(ar)
+    serializable_HashMap<zstring, V, serializable_HashMapZStringCmp>(ar)
   {
   }
 
   void serialize(::zorba::serialization::Archiver& ar)
   {
     serialize_baseclass(ar,
-    (serializable_HashMap<zstring, V, HashMapZStringCmp>*)this);
+    (serializable_HashMap<zstring, V, serializable_HashMapZStringCmp>*)this);
   }
 
 public:
   serializable_HashMapZString(ulong size, bool sync)
     :
-    serializable_HashMap<zstring, V, HashMapZStringCmp>(size, sync)
+    serializable_HashMap<zstring, V, serializable_HashMapZStringCmp>(size, sync)
   {
   }
 

=== added file 'src/zorbautils/hashmap_zstring_nonserializable.h'
--- src/zorbautils/hashmap_zstring_nonserializable.h	1970-01-01 00:00:00 +0000
+++ src/zorbautils/hashmap_zstring_nonserializable.h	2011-10-18 22:22:26 +0000
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2006-2008 The FLWOR Foundation.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+#ifndef ZORBA_HASHMAP_ZSTRING_NONSERIALIZABLE
+#define ZORBA_HASHMAP_ZSTRING_NONSERIALIZABLE
+
+#undef ZORBA_UTILS_HASHMAP_WITH_SERIALIZATION
+#include "zorbautils/hashmap.h"
+
+#include "zorbatypes/zstring.h"
+
+#include "util/utf8_util.h"
+
+
+namespace zorba
+{
+
+/***************************************************************************//**
+  Class to privide the equality and hash functions for the HashMapZString
+  class defined below.
+*******************************************************************************/
+class HashMapZStringCmp
+{
+public:
+  static uint32_t hash(const zstring& str)
+  {
+    return utf8::hash(str);
+  }
+
+  static bool equal(const zstring& s1, const zstring& s2)
+  {
+    return s1 == s2;
+  }
+};
+
+
+/*******************************************************************************
+  A nonserializable hash-based map container mapping zstrings to values of
+  type V. Equality is based on the zstring == operator.
+*******************************************************************************/
+template<class V>
+class HashMapZString : public HashMap<zstring, V, HashMapZStringCmp>
+{
+public:
+  HashMapZString()
+    :
+    HashMap<zstring, V, HashMapZStringCmp>()
+  {
+  }
+
+public:
+  HashMapZString(ulong size, bool sync)
+    :
+    HashMap<zstring, V, HashMapZStringCmp>(size, sync)
+  {
+  }
+
+  virtual ~HashMapZString() {  }
+};
+
+}
+
+#endif