When attempting to precalculated nodes during an expression prepare

stage, be more intelligent about compiling AND or OR nodes We can take advantage of the fact that and AND node will ALWAYS be false if either input node is static and evaluates to FALSE, and that OR nodes will always be true if either input is static and evaluates to TRUE. In some cases this allows us the shortcut and cut out non-static nodes during preparation, resulting in faster evaluation and more easily compiled expressions...
qgis · Feb 24, 2021 · 14f2ab5 · 14f2ab5
1 parent 66ab5af
commit 14f2ab5
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 9 deletions.
diff --git a/python/core/auto_generated/expression/qgsexpressionnode.sip.in b/python/core/auto_generated/expression/qgsexpressionnode.sip.in
@@ -283,6 +283,7 @@ Returns the node's static cached value. Only valid if :py:func:`~QgsExpressionNo
   protected:
 
 
+
 };
 
 

diff --git a/src/core/expression/qgsexpressionnode.cpp b/src/core/expression/qgsexpressionnode.cpp
@@ -32,18 +32,21 @@ QVariant QgsExpressionNode::eval( QgsExpression *parent, const QgsExpressionCont
 
 bool QgsExpressionNode::prepare( QgsExpression *parent, const QgsExpressionContext *context )
 {
+  mHasCachedValue = false;
   if ( isStatic( parent, context ) )
   {
-    mCachedStaticValue = evalNode( parent, context );
-    if ( !parent->hasEvalError() )
-      mHasCachedValue = true;
-    else
-      mHasCachedValue = false;
+    // some calls to isStatic already evaluate the node to a cached value, so if that's
+    // happened then don't re-evaluate again
+    if ( !mHasCachedValue )
+    {
+      mCachedStaticValue = evalNode( parent, context );
+      if ( !parent->hasEvalError() )
+        mHasCachedValue = true;
+    }
     return true;
   }
   else
   {
-    mHasCachedValue = false;
     return prepareNode( parent, context );
   }
 }

diff --git a/src/core/expression/qgsexpressionnode.h b/src/core/expression/qgsexpressionnode.h
@@ -342,6 +342,23 @@ class CORE_EXPORT QgsExpressionNode SIP_ABSTRACT
      */
     void cloneTo( QgsExpressionNode *target ) const SIP_SKIP;
 
+#ifndef SIP_RUN
+
+    /**
+     * TRUE if the node has a static, precalculated value.
+     *
+     * \since QGIS 3.20
+     */
+    mutable bool mHasCachedValue = false;
+
+    /**
+     * Contains the static, precalculated value for the node if mHasCachedValue is TRUE.
+     *
+     * \since QGIS 3.20
+     */
+    mutable QVariant mCachedStaticValue;
+#endif
+
   private:
 
     /**
@@ -358,8 +375,6 @@ class CORE_EXPORT QgsExpressionNode SIP_ABSTRACT
      */
     virtual QVariant evalNode( QgsExpression *parent, const QgsExpressionContext *context ) = 0;
 
-    bool mHasCachedValue = false;
-    QVariant mCachedStaticValue;
 };
 
 Q_DECLARE_METATYPE( QgsExpressionNode * )

diff --git a/src/core/expression/qgsexpressionnodeimpl.cpp b/src/core/expression/qgsexpressionnodeimpl.cpp
@@ -833,7 +833,112 @@ QgsExpressionNode *QgsExpressionNodeBinaryOperator::clone() const
 
 bool QgsExpressionNodeBinaryOperator::isStatic( QgsExpression *parent, const QgsExpressionContext *context ) const
 {
-  return mOpLeft->isStatic( parent, context ) && mOpRight->isStatic( parent, context );
+  const bool leftStatic = mOpLeft->isStatic( parent, context );
+  const bool rightStatic = mOpRight->isStatic( parent, context );
+
+  if ( leftStatic && rightStatic )
+    return true;
+
+  // special logic for certain ops...
+  switch ( mOp )
+  {
+    case QgsExpressionNodeBinaryOperator::boOr:
+    {
+      // if either node is static AND evaluates to TRUE, then the result will ALWAYS be true regardless
+      // of the value of the other node!
+      if ( leftStatic )
+      {
+        mOpLeft->prepare( parent, context );
+        if ( mOpLeft->hasCachedStaticValue() )
+        {
+          QgsExpressionUtils::TVL tvl = QgsExpressionUtils::getTVLValue( mOpLeft->cachedStaticValue(), parent );
+          if ( !parent->hasEvalError() && tvl == QgsExpressionUtils::True )
+          {
+            mCachedStaticValue = true;
+            mHasCachedValue = true;
+            return true;
+          }
+        }
+      }
+      else if ( rightStatic )
+      {
+        mOpRight->prepare( parent, context );
+        if ( mOpRight->hasCachedStaticValue() )
+        {
+          QgsExpressionUtils::TVL tvl = QgsExpressionUtils::getTVLValue( mOpRight->cachedStaticValue(), parent );
+          if ( !parent->hasEvalError() && tvl == QgsExpressionUtils::True )
+          {
+            mCachedStaticValue = true;
+            mHasCachedValue = true;
+            return true;
+          }
+        }
+      }
+
+      break;
+    }
+    case QgsExpressionNodeBinaryOperator::boAnd:
+    {
+      // if either node is static AND evaluates to FALSE, then the result will ALWAYS be false regardless
+      // of the value of the other node!
+
+      if ( leftStatic )
+      {
+        mOpLeft->prepare( parent, context );
+        if ( mOpLeft->hasCachedStaticValue() )
+        {
+          QgsExpressionUtils::TVL tvl = QgsExpressionUtils::getTVLValue( mOpLeft->cachedStaticValue(), parent );
+          if ( !parent->hasEvalError() && tvl == QgsExpressionUtils::False )
+          {
+            mCachedStaticValue = false;
+            mHasCachedValue = true;
+            return true;
+          }
+        }
+      }
+      else if ( rightStatic )
+      {
+        mOpRight->prepare( parent, context );
+        if ( mOpRight->hasCachedStaticValue() )
+        {
+          QgsExpressionUtils::TVL tvl = QgsExpressionUtils::getTVLValue( mOpRight->cachedStaticValue(), parent );
+          if ( !parent->hasEvalError() && tvl == QgsExpressionUtils::False )
+          {
+            mCachedStaticValue = false;
+            mHasCachedValue = true;
+            return true;
+          }
+        }
+      }
+
+      break;
+    }
+
+    case QgsExpressionNodeBinaryOperator::boEQ:
+    case QgsExpressionNodeBinaryOperator::boNE:
+    case QgsExpressionNodeBinaryOperator::boLE:
+    case QgsExpressionNodeBinaryOperator::boGE:
+    case QgsExpressionNodeBinaryOperator::boLT:
+    case QgsExpressionNodeBinaryOperator::boGT:
+    case QgsExpressionNodeBinaryOperator::boRegexp:
+    case QgsExpressionNodeBinaryOperator::boLike:
+    case QgsExpressionNodeBinaryOperator::boNotLike:
+    case QgsExpressionNodeBinaryOperator::boILike:
+    case QgsExpressionNodeBinaryOperator::boNotILike:
+    case QgsExpressionNodeBinaryOperator::boIs:
+    case QgsExpressionNodeBinaryOperator::boIsNot:
+    case QgsExpressionNodeBinaryOperator::boPlus:
+    case QgsExpressionNodeBinaryOperator::boMinus:
+    case QgsExpressionNodeBinaryOperator::boMul:
+    case QgsExpressionNodeBinaryOperator::boDiv:
+    case QgsExpressionNodeBinaryOperator::boIntDiv:
+    case QgsExpressionNodeBinaryOperator::boMod:
+    case QgsExpressionNodeBinaryOperator::boPow:
+    case QgsExpressionNodeBinaryOperator::boConcat:
+      break;
+  }
+
+  return false;
 }
 
 //