HPCC-15610 Reduce optimization level for very complicated functions

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ecl/hqlcpp/hqlckey.cpp

@@ -110,9 +110,9 @@ void HqlCppTranslator::buildJoinMatchFunction(BuildCtx & ctx, const char * name,
     if (match)
     {
         StringBuffer proto;
-        proto.append("virtual bool ").append(name).append("(const void * _left, const void * _right) override" OPTIMIZE_FUNCTION_ATTRIBUTE);
+        proto.append("virtual bool ").append(name).append("(const void * _left, const void * _right) override");
 
-        MemberFunction matchFunc(*this, ctx, proto, MFdynamicproto);
+        MemberFunction matchFunc(*this, ctx, proto, MFdynamicproto|MFoptimize);
 
         matchFunc.ctx.addQuotedLiteral("const unsigned char * left = (const unsigned char *) _left;");
         matchFunc.ctx.addQuotedLiteral("const unsigned char * right = (const unsigned char *) _right;");

ecl/hqlcpp/hqlcpp.cpp

@@ -1858,6 +1858,8 @@ void HqlCppTranslator::cacheOptions()
         DebugOption(options.genericDiskReads, "genericDiskReads", false),
         DebugOption(options.generateActivityFormats, "generateActivityFormats", false),
         DebugOption(options.generateDiskFormats, "generateDiskFormats", false),
+        DebugOption(options.maxOptimizeSize, "maxOptimizeSize", 5),             // Remove the overhead from very small functions e.g. function prolog
+        DebugOption(options.minNoOptimizeSize, "minNoOptimizeSize", 10000),     // functions larger than this will take a long time to optimize, better to not try
     };
 
     //get options values from workunit

ecl/hqlcpp/hqlcpp.ipp

@@ -36,7 +36,6 @@
 #endif
 
 #define MAX_RECORD_SIZE     4096                // default value
-#define OPTIMIZE_FUNCTION_ATTRIBUTE " OPTIMIZE"
 
 enum GraphLocalisation {
     GraphNeverAccess,  // This variant of an activity never accesses the parent
@@ -442,11 +441,13 @@ protected:
 
 //===========================================================================
 
-enum
+enum MFoptions
 {
     MFdynamicproto = 1,             // Prototype for the function is not a literal string
     MFsingle = 2,                   // This will only be executed once per activity instance
     MFopt = 4,                      // An optional function that will not be generated if it is empty
+    MFoptimize = 8,                 // Ensure this function is optimized as well as possible
+    MFnooptimize = 16,              // Disable optimization - the function is too complicated
 };
 
 class MemberFunction
@@ -629,6 +630,8 @@ struct HqlCppOptions
     unsigned            checkDuplicateMinActivities;
     CompilerType        targetCompiler;
     DBZaction           divideByZeroAction;
+    unsigned            maxOptimizeSize;
+    unsigned            minNoOptimizeSize;
     bool                peephole;
     bool                foldConstantCast;
     bool                optimizeBoolReturn;

ecl/hqlcpp/hqlhtcpp.cpp

@@ -265,10 +265,26 @@ MemberFunction::MemberFunction(HqlCppTranslator & _translator, BuildCtx & classc
 MemberFunction::MemberFunction(HqlCppTranslator & _translator, BuildCtx & classctx, const char * text, unsigned _flags) : translator(_translator), ctx(classctx), flags(_flags)
 {
     stmt = ctx.addQuotedFunction(text, (flags & MFdynamicproto) != 0);
+    if (flags & MFoptimize)
+        stmt->setOptimize(true);
+    else if (flags & MFnooptimize)
+        stmt->setOptimize(false);
 }
 
 MemberFunction::~MemberFunction() noexcept(false)
 {
+    unsigned maxOptimizeSize = translator.queryOptions().maxOptimizeSize;
+    unsigned minNoOptimizeSize = translator.queryOptions().minNoOptimizeSize;
+    if (stmt && (maxOptimizeSize || minNoOptimizeSize))
+    {
+        //This estimate is before the peephole optimize is applied, so may be a large over estimate
+        unsigned estimatedSize = calcTotalChildren(stmt);
+        if (maxOptimizeSize && (estimatedSize <= maxOptimizeSize))
+            stmt->setOptimize(true);
+        else if (minNoOptimizeSize && (estimatedSize >= minNoOptimizeSize))
+            stmt->setOptimize(false);
+    }
+
     //Do not process the aliases if we are aborting from an error
     if (!std::uncaught_exception())
         finish();
@@ -5529,7 +5545,7 @@ void HqlCppTranslator::buildCompareClass(BuildCtx & ctx, const char * name, IHql
     IHqlStmt * classStmt = beginNestedClass(classctx, name, "ICompare");
 
     {
-        MemberFunction func(*this, classctx, "virtual int docompare(const void * _left, const void * _right) const override" OPTIMIZE_FUNCTION_ATTRIBUTE);
+        MemberFunction func(*this, classctx, "virtual int docompare(const void * _left, const void * _right) const override", MFoptimize);
         func.ctx.addQuotedLiteral("const unsigned char * left = (const unsigned char *) _left;");
         func.ctx.addQuotedLiteral("const unsigned char * right = (const unsigned char *) _right;");
         func.ctx.associateExpr(constantMemberMarkerExpr, constantMemberMarkerExpr);
@@ -5689,7 +5705,7 @@ void HqlCppTranslator::buildHashClass(BuildCtx & ctx, const char * name, IHqlExp
 
 static void buildCompareMemberFunction(HqlCppTranslator & translator, BuildCtx & ctx, IHqlExpression * sortList, const DatasetReference & dataset)
 {
-    MemberFunction func(translator, ctx, "virtual int docompare(const void * _left, const void * _right) const override" OPTIMIZE_FUNCTION_ATTRIBUTE);
+    MemberFunction func(translator, ctx, "virtual int docompare(const void * _left, const void * _right) const override", MFoptimize);
     func.ctx.addQuotedLiteral("const unsigned char * left = (const unsigned char *) _left;");
     func.ctx.addQuotedLiteral("const unsigned char * right = (const unsigned char *) _right;");
     func.ctx.associateExpr(constantMemberMarkerExpr, constantMemberMarkerExpr);
@@ -11943,7 +11959,7 @@ void HqlCppTranslator::generateSortCompare(BuildCtx & nestedctx, BuildCtx & ctx,
         IHqlStmt * classStmt = beginNestedClass(classctx, compareName.str(), "ICompare");
 
         {
-            MemberFunction func(*this, classctx, "virtual int docompare(const void * _left, const void * _right) const override" OPTIMIZE_FUNCTION_ATTRIBUTE);
+            MemberFunction func(*this, classctx, "virtual int docompare(const void * _left, const void * _right) const override", MFoptimize);
             func.ctx.addQuotedLiteral("const unsigned char * left = (const unsigned char *) _left;");
             func.ctx.addQuotedLiteral("const unsigned char * right = (const unsigned char *) _right;");
             func.ctx.associateExpr(constantMemberMarkerExpr, constantMemberMarkerExpr);
@@ -18543,23 +18559,18 @@ void HqlCppTranslator::buildWorkflow(WorkflowArray & workflow)
 {
     //Generate a #define that can be used to optimize a particular function.
     BuildCtx optimizectx(*code, includeAtom);
-    if (options.optimizeCriticalFunctions)
+    switch (options.targetCompiler)
     {
-        switch (options.targetCompiler)
-        {
 #ifndef __APPLE__
-        case GccCppCompiler:
-            optimizectx.addQuoted("#define OPTIMIZE __attribute__((optimize(3)))");
-            break;
+    case GccCppCompiler:
+        optimizectx.addQuoted("#define NOOPTIMIZE __attribute__((optimize(0)))");
+        optimizectx.addQuoted("#define OPTIMIZE __attribute__((optimize(3)))");
+        break;
 #endif
-        default:
-            optimizectx.addQuoted("#define OPTIMIZE");
-            break;
-        }
-    }
-    else
-    {
+    default:
+        optimizectx.addQuoted("#define NOOPTIMIZE");
         optimizectx.addQuoted("#define OPTIMIZE");
+        break;
     }
 
 

ecl/hqlcpp/hqlstmt.cpp

@@ -1215,6 +1215,8 @@ HqlStmt::HqlStmt(StmtKind _kind, HqlStmts * _container)
     incomplete = false;
     included = true;
     priority = 0;
+    optimize = false;
+    noOptimize = false;
 }
 
 void HqlStmt::addExpr(IHqlExpression * expr)
@@ -1247,6 +1249,16 @@ static bool isEmptyGroup(IHqlStmt * stmt)
     return false;
 }
 
+StringBuffer & HqlStmt::appendTextExtra(StringBuffer & out) const
+{
+    if (noOptimize)
+        out.append(" NOOPTIMIZE");
+    if (optimize)
+        out.append(" OPTIMIZE");
+    return out;
+}
+
+
 bool HqlStmt::hasChildren() const
 {
     if (numChildren() == 0)
@@ -1301,6 +1313,12 @@ IHqlExpression * HqlStmt::queryExpr(unsigned index) const
     return NULL;
 }
 
+void HqlStmt::setOptimize(bool value)
+{
+    optimize = value;
+    noOptimize = !value;
+}
+
 
 //---------------------------------------------------------------------------
 
@@ -1371,13 +1389,13 @@ StringBuffer & HqlQuoteLiteralStmt::getTextExtra(StringBuffer & out) const
 
 StringBuffer & HqlQuoteCompoundStmt::getTextExtra(StringBuffer & out) const
 {
-    return out.append(text);
+    return appendTextExtra(out.append(text));
 }
 
 
 StringBuffer & HqlQuoteLiteralCompoundStmt::getTextExtra(StringBuffer & out) const
 {
-    return out.append(text);
+    return appendTextExtra(out.append(text));
 }
 
 

ecl/hqlcpp/hqlstmt.hpp

@@ -185,29 +185,30 @@ protected:
 };
 
 
-enum StmtKind { 
-             null_stmt,
-             assign_stmt, block_stmt, group_stmt, declare_stmt, 
-             expr_stmt,
-             return_stmt,
-             quote_stmt,
-             quote_compound_stmt,
-             quote_compoundopt_stmt,
-             filter_stmt,
-             goto_stmt, label_stmt,
-             switch_stmt, case_stmt, default_stmt,
-             loop_stmt, break_stmt,
-             pass_stmt, external_stmt,
-             indirect_stmt,
-             assigninc_stmt, assigndec_stmt,
-             alias_stmt,
-             line_stmt,
-             continue_stmt,
-             function_stmt,
-             assign_link_stmt,
-             try_stmt,
-             catch_stmt,
-             throw_stmt,
+enum StmtKind : byte
+{
+     null_stmt,
+     assign_stmt, block_stmt, group_stmt, declare_stmt,
+     expr_stmt,
+     return_stmt,
+     quote_stmt,
+     quote_compound_stmt,
+     quote_compoundopt_stmt,
+     filter_stmt,
+     goto_stmt, label_stmt,
+     switch_stmt, case_stmt, default_stmt,
+     loop_stmt, break_stmt,
+     pass_stmt, external_stmt,
+     indirect_stmt,
+     assigninc_stmt, assigndec_stmt,
+     alias_stmt,
+     line_stmt,
+     continue_stmt,
+     function_stmt,
+     assign_link_stmt,
+     try_stmt,
+     catch_stmt,
+     throw_stmt,
 };
 
 
@@ -225,6 +226,7 @@ public:
     virtual void            mergeScopeWithContainer() = 0;
     virtual void            setIncomplete(bool incomplete) = 0;
     virtual void            setIncluded(bool _included) = 0;
+    virtual void            setOptimize(bool value) = 0;
     virtual void            finishedFramework() = 0;
 };
 

ecl/hqlcpp/hqlstmt.ipp

@@ -45,21 +45,21 @@ public:
     virtual void                    mergeScopeWithContainer()       {}
     virtual void                    setIncomplete(bool _incomplete) { incomplete = _incomplete; }
     virtual void                    setIncluded(bool _included) { included = _included; }
+    virtual void                    setOptimize(bool value);
             void                    setPriority(unsigned _prio) { priority = _prio; }
     virtual void                    finishedFramework() { throwUnexpected(); }
 
 protected:
     bool hasChildren() const;
+    StringBuffer & appendTextExtra(StringBuffer & out) const;
 
 protected:
     unsigned short                      priority;       //64bit: pack with link count in CInterface
-#ifdef _DEBUG
     StmtKind                            kind;
-#else
-    unsigned char                       kind;
-#endif
     bool                                incomplete:1;
     bool                                included:1;
+    bool                                optimize:1;     // Should really be in a derived class, but this avoids extra memory
+    byte                                noOptimize:1;   // Should really be in a derived class, but this avoids extra memory
     HqlStmts *                          container;
     HqlExprArray                        exprs;
 };