Markos Zaharioudakis has proposed merging lp:~zorba-coders/zorba/markos-scratch 
into lp:zorba.

Requested reviews:
  Markos Zaharioudakis (markos-za)

For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/markos-scratch/+merge/92871

1. Optimization: preallocate and reuse temp sequence iterator for 
LetVarITerator and CtxVarIterator.
2.  Optimization: enhanced push-down of positional predicate into 
LetVarITerator and CtxVarIterator.
-- 
https://code.launchpad.net/~zorba-coders/zorba/markos-scratch/+merge/92871
Your team Zorba Coders is subscribed to branch lp:zorba.
=== modified file 'ChangeLog'
--- ChangeLog	2012-02-10 01:01:16 +0000
+++ ChangeLog	2012-02-13 22:55:21 +0000
@@ -26,6 +26,10 @@
   * Added split function to the string module that allows for streamable tokenization but doesn't have regular expression
     support.
   * Fixed bug involving positional var and groupby 
+  * Optimization: preallocate and reuse temp sequence iterator for LetVarITerator
+    and CtxVarIterator.
+  * Optimization: enhanced push-down of positional predicate into LetVarITerator
+    and CtxVarIterator.
   * zerr is not predeclared anymore to be http://www.zorba-xquery.com/errors
   * Add new XQuery interface for the PHP bindings.
   * Added API method Item::getNamespaceBindings().

=== modified file 'src/functions/func_sequences_impl.cpp'
--- src/functions/func_sequences_impl.cpp	2012-01-23 10:06:24 +0000
+++ src/functions/func_sequences_impl.cpp	2012-02-13 22:55:21 +0000
@@ -393,12 +393,11 @@
     const var_expr* inputVar = inputExpr->get_var();
 
     if (inputVar != NULL &&
-        lenExpr != NULL &&
         (inputVar->get_kind() == var_expr::let_var ||
          inputVar->get_kind() == var_expr::win_var ||
          inputVar->get_kind() == var_expr::non_groupby_var) &&
         letVarIter->setTargetPosIter(aArgs[1]) &&
-        letVarIter->setTargetLenIter(aArgs[2]))
+        letVarIter->setTargetLenIter(lenExpr ? aArgs[2] : NULL))
     {
       return aArgs[0];
     }
@@ -408,10 +407,9 @@
     const var_expr* inputVar = inputExpr->get_var();
 
     if (inputVar != NULL &&
-        lenExpr != NULL &&
         !inputVar->is_context_item() &&
         ctxVarIter->setTargetPosIter(aArgs[1]) &&
-        ctxVarIter->setTargetLenIter(aArgs[2]))
+        ctxVarIter->setTargetLenIter(lenExpr ? aArgs[2] : NULL))
     {
       return aArgs[0];
     }

=== modified file 'src/runtime/core/var_iterators.cpp'
--- src/runtime/core/var_iterators.cpp	2011-12-21 14:40:33 +0000
+++ src/runtime/core/var_iterators.cpp	2012-02-13 22:55:21 +0000
@@ -29,9 +29,11 @@
 #include "runtime/visitors/planiter_visitor.h"
 
 #include "store/api/iterator.h"
+#include "store/api/iterator_factory.h"
 #include "store/api/item.h"
 #include "store/api/item_factory.h"
 #include "store/api/temp_seq.h"
+#include "store/api/store.h"
 
 #include "util/string_util.h"
 
@@ -262,6 +264,9 @@
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 CtxVarIterator::CtxVarIterator(
     static_context* sctx,
     const QueryLoc& loc,
@@ -273,7 +278,8 @@
   theVarId(varid),
   theVarName(varName),
   theIsLocal(isLocal),
-  theTargetPos(0)
+  theTargetPos(0),
+  theInfLen(false)
 {
 }
 
@@ -289,6 +295,7 @@
   ar & theTargetPos;
   ar & theTargetPosIter;
   ar & theTargetLenIter;
+  ar & theInfLen;
 }
 
 
@@ -316,15 +323,24 @@
 
 bool CtxVarIterator::setTargetLenIter(const PlanIter_t& v)
 {
-  if (theTargetPos == Integer(0) && theTargetLenIter == NULL)
+  if (theTargetPos == Integer(0) &&
+      theTargetLenIter == NULL &&
+      theInfLen == false)
   {
-    theTargetLenIter = v;
+    if (v == NULL)
+      theInfLen = true;
+    else
+      theTargetLenIter = v;
+
     return true;
   }
   return false;
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 uint32_t CtxVarIterator::getStateSizeOfSubtree() const
 {
   uint32_t size = 0;
@@ -345,6 +361,9 @@
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 void CtxVarIterator::openImpl(
     PlanState& planState,
     uint32_t& offset)
@@ -361,6 +380,9 @@
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 void CtxVarIterator::resetImpl(PlanState& planState) const
 {
   NoaryBaseIterator<CtxVarIterator, CtxVarState>::resetImpl(planState);
@@ -375,6 +397,9 @@
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 void CtxVarIterator::closeImpl(PlanState& planState)
 {
   NoaryBaseIterator<CtxVarIterator, CtxVarState>::closeImpl(planState);
@@ -389,7 +414,9 @@
 }
 
 
+/*******************************************************************************
 
+********************************************************************************/
 bool CtxVarIterator::nextImpl(store::Item_t& result, PlanState& planState) const
 {
   store::Item_t varSingleItem;
@@ -405,7 +432,7 @@
   CtxVarState* state;
   DEFAULT_STACK_INIT(CtxVarState, state, planState);
 
-  if (theTargetPosIter != NULL && theTargetLenIter == NULL)
+  if (theTargetPosIter != NULL && theTargetLenIter == NULL && theInfLen == false)
   {
     result = NULL;
 
@@ -443,7 +470,7 @@
     }
   } // if (theTargetPosIter != NULL && theTargetLenIter == NULL)
 
-  else if (theTargetPosIter != NULL && theTargetLenIter != NULL)
+  else if (theTargetPosIter != NULL)
   {
     result = NULL;
 
@@ -454,12 +481,24 @@
 
     startPos = posItem->getIntegerValue();
 
-    if (!consumeNext(lenItem, theTargetLenIter, planState))
-    {
-      ZORBA_ASSERT(false);
-    }
+    dctx->get_variable(theVarId, theVarName, loc, varSingleItem, state->theTempSeq);
 
-    len = lenItem->getIntegerValue();
+    if (theInfLen)
+    {
+      if (varSingleItem != NULL)
+        len = 1;
+      else
+        len = state->theTempSeq->getSize();
+    }
+    else
+    {
+      if (!consumeNext(lenItem, theTargetLenIter, planState))
+      {
+        ZORBA_ASSERT(false);
+      }
+      
+      len = lenItem->getIntegerValue();
+    }
 
     if (startPos <= Integer(0))
     {
@@ -467,10 +506,8 @@
       startPos = 1;
     }
 
-    state->theLastPos = startPos + len;
-    state->thePos = startPos;
-
-    dctx->get_variable(theVarId, theVarName, loc, varSingleItem, state->theTempSeq);
+      state->theLastPos = startPos + len;
+      state->thePos = startPos;
 
     if (varSingleItem != NULL)
     {
@@ -542,7 +579,10 @@
     }
     else if (state->theTempSeq != NULL)
     {
-      state->theSourceIter = state->theTempSeq->getIterator();
+      if (state->theSourceIter == NULL)
+        state->theSourceIter = GENV_STORE.getIteratorFactory()->createTempSeqIterator();
+
+      state->theSourceIter->init(state->theTempSeq);
       state->theSourceIter->open();
 
       while (state->theSourceIter->next(result))
@@ -551,7 +591,6 @@
       }
 
       state->theSourceIter->close();
-      state->theSourceIter = NULL;
     }
     else
     {
@@ -648,9 +687,15 @@
 
   if (theSourceIter != NULL)
     theSourceIter->reset();
+
+  if (theTempSeqIter != NULL)
+    theTempSeqIter->reset();
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 LetVarIterator::LetVarIterator(
     static_context* sctx,
     const QueryLoc& loc,
@@ -658,7 +703,8 @@
   :
   NoaryBaseIterator<LetVarIterator, LetVarState>(sctx, loc),
   theVarName(name),
-  theTargetPos(0)
+  theTargetPos(0),
+  theInfLen(false)
 {
 }
 
@@ -670,9 +716,13 @@
   ar & theTargetPos;
   ar & theTargetPosIter;
   ar & theTargetLenIter;
+  ar & theInfLen;
 }
 
 
+/*******************************************************************************
+
+********************************************************************************/
 bool LetVarIterator::setTargetPos(xs_integer v)
 {
   if (theTargetPos == Integer(0) && theTargetPosIter == NULL)
@@ -697,9 +747,15 @@
 
 bool LetVarIterator::setTargetLenIter(const PlanIter_t& v)
 {
-  if (theTargetPos == Integer(0) && theTargetLenIter == NULL)
+  if (theTargetPos == Integer(0) &&
+      theTargetLenIter == NULL && 
+      theInfLen == false)
   {
-    theTargetLenIter = v;
+    if (v == NULL)
+      theInfLen = true;
+    else
+      theTargetLenIter = v;
+
     return true;
   }
   return false;
@@ -721,7 +777,7 @@
 /*******************************************************************************
 
 ********************************************************************************/
-void LetVarIterator::bind(store::TempSeq_t& value, PlanState& planState)
+void LetVarIterator::bind(const store::TempSeq_t& value, PlanState& planState)
 {
   LetVarState* state;
   state = StateTraitsImpl<LetVarState>::getState(planState, theStateOffset);
@@ -736,8 +792,11 @@
     }
     else
     {
-      state->theSourceIter = state->theTempSeq->getIterator();
-      state->theSourceIter->open();
+      if (state->theTempSeqIter == NULL)
+        state->theTempSeqIter = GENV_STORE.getIteratorFactory()->createTempSeqIterator();
+
+      state->theTempSeqIter->init(value);
+      state->theTempSeqIter->open();
     }
   }
 }
@@ -749,8 +808,8 @@
 void LetVarIterator::bind(
     store::TempSeq_t& value,
     PlanState& planState,
-    ulong startPos,
-    ulong endPos)
+    xs_integer startPos,
+    xs_integer endPos)
 {
   LetVarState* state;
   state = StateTraitsImpl<LetVarState>::getState(planState, theStateOffset);
@@ -761,12 +820,15 @@
   {
     if (theTargetPos > Integer(0))
     {
-      value->getItem((Integer(startPos) + theTargetPos - Integer(1)), state->theItem);
+      value->getItem((startPos + theTargetPos - Integer(1)), state->theItem);
     }
     else
     {
-      state->theSourceIter = state->theTempSeq->getIterator(startPos, endPos, true);
-      state->theSourceIter->open();
+      if (state->theTempSeqIter == NULL)
+        state->theTempSeqIter = GENV_STORE.getIteratorFactory()->createTempSeqIterator();
+
+      state->theTempSeqIter->init(value, startPos, endPos);
+      state->theTempSeqIter->open();
     }
   }
 }
@@ -849,7 +911,7 @@
 
     startPos = posItem->getIntegerValue();
 
-    if (theTargetLenIter == NULL)
+    if (theTargetLenIter == NULL && theInfLen == false)
     {
       if (startPos > Integer(0))
         state->theTempSeq->getItem(startPos, result);
@@ -859,12 +921,19 @@
     }
     else
     {
-      if (!consumeNext(lenItem, theTargetLenIter, planState))
+      if (theInfLen)
       {
-        ZORBA_ASSERT(false);
+        len = state->theTempSeq->getSize();
       }
+      else
+      {
+        if (!consumeNext(lenItem, theTargetLenIter, planState))
+        {
+          ZORBA_ASSERT(false);
+        }
 
-      len = lenItem->getIntegerValue();
+        len = lenItem->getIntegerValue();
+      }
 
       if (startPos <= Integer(0))
       {
@@ -892,9 +961,17 @@
     if (result)
       STACK_PUSH(true, state);
   }
+  else if (state->theTempSeqIter)
+  {
+    assert(state->theSourceIter == NULL);
+    while (state->theTempSeqIter->next(result))
+    {
+      STACK_PUSH(true, state);
+    }
+  }
   else
   {
-    assert(state->theSourceIter != NULL);
+    assert(state->theSourceIter != NULL && state->theTempSeqIter == NULL);
     while (state->theSourceIter->next(result))
     {
       STACK_PUSH(true, state);

=== modified file 'src/runtime/core/var_iterators.h'
--- src/runtime/core/var_iterators.h	2011-12-21 14:40:33 +0000
+++ src/runtime/core/var_iterators.h	2012-02-13 22:55:21 +0000
@@ -223,13 +223,13 @@
 class CtxVarState : public  PlanIteratorState 
 {
 public:
-  store::TempSeq_t  theTempSeq;
-
-  store::Iterator_t theSourceIter;
-  store::Item_t     theItem;
-
-  xs_integer        thePos;
-  xs_integer        theLastPos;
+  store::TempSeq_t         theTempSeq;
+
+  store::TempSeqIterator_t theSourceIter;
+  store::Item_t            theItem;
+
+  xs_integer               thePos;
+  xs_integer               theLastPos;
 
 public:
   CtxVarState();
@@ -250,6 +250,7 @@
   xs_integer     theTargetPos;
   PlanIter_t     theTargetPosIter;
   PlanIter_t     theTargetLenIter;
+  bool           theInfLen;
 
 public:
   SERIALIZABLE_CLASS(CtxVarIterator);
@@ -366,22 +367,34 @@
 
   LET variables. A LetVarIterator represents a reference to a let variable V.
   
-  theSourceIter stores the current "value" of V: it is a PlanIteratorWraper
-  that may wrap the actual domain expression that defines the var, or an
-  iterator over a temp sequence, if the result of the domain expression is
-  materialized.
+  If the var is materialized, then its value is stored in a temp sequence, and a
+  (smart) pointer to that temp seq is stored in the state of the LetVarIterator.
+  Otherwise, it's value is a PlanIteratorWraper that may wrap the actual domain 
+  expression that defines the var.
+
+  theSourceIter:
+  --------------
+  Stores the current "value" of V, if the variable is not materialized.
+
+  theTempSeq:
+  -----------
+  The temp seq storing the value of tha variable.
+ 
+  theTempSeqIter:
+  ---------------
+  A pre-allocated iterator over the temp seq.
 
 ********************************************************************************/
 class LetVarState : public PlanIteratorState 
 {
 public:
-  store::TempSeq_t  theTempSeq;
-
-  store::Iterator_t theSourceIter;
-  store::Item_t     theItem;
-
-  xs_integer        thePos;
-  xs_integer        theLastPos;
+  store::Iterator_t        theSourceIter;
+
+  store::TempSeq_t         theTempSeq;
+  store::TempSeqIterator_t theTempSeqIter;
+  store::Item_t            theItem;
+  xs_integer               thePos;
+  xs_integer               theLastPos;
 
 public:
   LetVarState();
@@ -399,6 +412,7 @@
   xs_integer     theTargetPos;
   PlanIter_t     theTargetPosIter;
   PlanIter_t     theTargetLenIter;
+  bool           theInfLen;
 
 public:
   SERIALIZABLE_CLASS(LetVarIterator);
@@ -431,9 +445,13 @@
 
   store::Item* getVarName() const { return theVarName.getp(); }
 
-  void bind(store::TempSeq_t& value, PlanState& planState);
+  void bind(const store::TempSeq_t& value, PlanState& planState);
 
-  void bind(store::TempSeq_t& value, PlanState& planState, ulong startPos, ulong endPos);
+  void bind(
+      store::TempSeq_t& value,
+      PlanState& planState,
+      xs_integer startPos,
+      xs_integer endPos);
 
   void bind(store::Iterator_t& it, PlanState& planState);
 

=== modified file 'src/store/api/iterator.h'
--- src/store/api/iterator.h	2011-06-14 17:26:33 +0000
+++ src/store/api/iterator.h	2012-02-13 22:55:21 +0000
@@ -208,6 +208,8 @@
 
   virtual void init(const TempSeq_t& seq) = 0;
   
+  virtual void init(const TempSeq_t& seq, xs_integer startPos, xs_integer endPos) = 0;
+
   virtual void open() = 0;
   
   virtual Item* next() = 0;

=== modified file 'src/store/api/temp_seq.h'
--- src/store/api/temp_seq.h	2011-12-21 14:40:33 +0000
+++ src/store/api/temp_seq.h	2012-02-13 22:55:21 +0000
@@ -80,7 +80,6 @@
    */
   virtual void getItem(xs_integer position, Item_t& result) = 0;
 		
-		
   /**
    * Returns true if the item at the passed position is available.
    * 
@@ -92,6 +91,11 @@
   virtual bool containsItem(xs_integer position) = 0;
 
   /**
+   *
+   */
+  virtual xs_integer getSize() const = 0;
+
+  /**
    * Reads the whole Sequence from beginning to end; it is allowed to have several 
    * concurrent iterators on the same TempSeq.
    * 

=== modified file 'src/store/naive/simple_lazy_temp_seq.cpp'
--- src/store/naive/simple_lazy_temp_seq.cpp	2012-02-02 14:42:18 +0000
+++ src/store/naive/simple_lazy_temp_seq.cpp	2012-02-13 22:55:21 +0000
@@ -233,6 +233,16 @@
 
 
 /*******************************************************************************
+
+********************************************************************************/
+xs_integer SimpleLazyTempSeq::getSize() const
+{
+  ZORBA_ASSERT(false);
+  return 0;
+}
+
+
+/*******************************************************************************
   Reads the whole Sequence from beginning to end; it is allowed to have several
   concurrent iterators on the same TempSeq.
 

=== modified file 'src/store/naive/simple_lazy_temp_seq.h'
--- src/store/naive/simple_lazy_temp_seq.h	2011-12-21 14:40:33 +0000
+++ src/store/naive/simple_lazy_temp_seq.h	2012-02-13 22:55:21 +0000
@@ -76,6 +76,8 @@
 
   bool containsItem(xs_integer position);
 
+  xs_integer getSize() const;
+
   store::Iterator_t getIterator() const;
 
   store::Iterator_t getIterator(

=== modified file 'src/store/naive/simple_temp_seq.cpp'
--- src/store/naive/simple_temp_seq.cpp	2012-02-02 14:42:18 +0000
+++ src/store/naive/simple_temp_seq.cpp	2012-02-13 22:55:21 +0000
@@ -226,13 +226,22 @@
 
 
 SimpleTempSeqIter::SimpleTempSeqIter(
-    const SimpleTempSeq* aTempSeq,
-    xs_integer startPos,
-    xs_integer endPos)
-	:
-  theTempSeq(const_cast<SimpleTempSeq*>(aTempSeq)),
-  theBorderType(startEnd)
-{
+    const SimpleTempSeq* seq,
+    xs_integer startPos,
+    xs_integer endPos)
+{
+  init(const_cast<SimpleTempSeq*>(seq), startPos, endPos);
+}
+
+
+void SimpleTempSeqIter::init(
+    const store::TempSeq_t& aTempSeq,
+    xs_integer startPos,
+    xs_integer endPos)
+{
+  theTempSeq = aTempSeq;
+  theBorderType = startEnd;
+
   xs_long start;
   xs_long end;
 

=== modified file 'src/store/naive/simple_temp_seq.h'
--- src/store/naive/simple_temp_seq.h	2011-12-21 14:40:33 +0000
+++ src/store/naive/simple_temp_seq.h	2012-02-13 22:55:21 +0000
@@ -24,14 +24,14 @@
 namespace zorba { namespace simplestore {
 
 /**
- * Very simple implementation of Temp Sequence. It saves the resulting items
- * of an iterator eager in a vector.
+ * 
  */
 typedef rchandle<class SimpleTempSeq> SimpleTempSeq_t;
 
 
 /*******************************************************************************
-
+  Very simple implementation of Temp Sequence. It eagerly saves the items
+  returned by an iterator into a vector.
 ********************************************************************************/
 class SimpleTempSeq : public store::TempSeq
 {
@@ -108,17 +108,16 @@
  public:
   SimpleTempSeqIter() {}
 
-  SimpleTempSeqIter(const SimpleTempSeq* aTempSeq);
+  SimpleTempSeqIter(const SimpleTempSeq* seq);
 
-  SimpleTempSeqIter(
-      const SimpleTempSeq* aTempSeq,
-      xs_integer startPos,
-      xs_integer endPos);
+  SimpleTempSeqIter(const SimpleTempSeq* seq, xs_integer startPos, xs_integer endPos);
 
   ~SimpleTempSeqIter() {}
 
   void init(const store::TempSeq_t& seq);
 
+  void init(const store::TempSeq_t& seq, xs_integer startPos, xs_integer endPos);
+
   store::Item* next();
 
   void open();

=== modified file 'test/rbkt/testdriver.cpp'
--- test/rbkt/testdriver.cpp	2011-07-16 23:46:39 +0000
+++ test/rbkt/testdriver.cpp	2012-02-13 22:55:21 +0000
@@ -26,6 +26,7 @@
 #include <time.h>
 #endif
 
+//#define ZORBA_TEST_PLAN_SERIALIZATION
 
 #include "testdriverconfig.h" // SRC and BIN dir definitions
 #include "specification.h" // parsing spec files

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to     : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp

Reply via email to