Browse Source

HPCC-17137 Improve efficiency of registerTimer

No need to look up the activity - it is always the current one.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>
Richard Chapman 8 years ago
parent
commit
73af921e29

+ 4 - 0
common/thorhelper/thorcommon.hpp

@@ -577,6 +577,10 @@ public:
     {
         return ctx->registerTimer(activityId, name);
     }
+    virtual unsigned getGraphLoopCounter() const override
+    {
+        return ctx->getGraphLoopCounter();
+    }
 protected:
     ICodeContext * ctx;
 };

+ 1 - 0
ecl/eclagent/eclagent.ipp

@@ -602,6 +602,7 @@ public:
     void addTimings();
 
 // ICodeContext
+    virtual unsigned getGraphLoopCounter() const override { return 0; }
     virtual unsigned getNodes();
     virtual unsigned getNodeNum() { return 0; }
     virtual char *getFilePart(const char *logicalPart, bool create=false);

+ 4 - 0
roxie/ccd/ccdactivities.cpp

@@ -625,6 +625,10 @@ public:
     }
     virtual IEngineContext *queryEngineContext() { return NULL; }
     virtual IWorkUnit *updateWorkUnit() const { throwUnexpected(); }
+    virtual unsigned getGraphLoopCounter() const override { return queryContext->queryCodeContext()->getGraphLoopCounter(); }
+    virtual IEclGraphResults * resolveLocalQuery(__int64 activityId) override { return queryContext->queryCodeContext()->resolveLocalQuery(activityId); }
+    virtual IDebuggableContext *queryDebugContext() const override { return queryContext->queryCodeContext()->queryDebugContext(); }
+
 };
 
 //================================================================================================

+ 1 - 6
roxie/ccd/ccdcontext.cpp

@@ -1656,6 +1656,7 @@ public:
 
     virtual unsigned __int64 getDatasetHash(const char * name, unsigned __int64 hash) { throwUnexpected(); }
 
+    virtual unsigned getGraphLoopCounter() const override { return 0; }
     virtual unsigned getNodes() { throwUnexpected(); }
     virtual unsigned getNodeNum() { throwUnexpected(); }
     virtual char *getFilePart(const char *logicalPart, bool create=false) { throwUnexpected(); }
@@ -1982,12 +1983,6 @@ public:
     virtual ISectionTimer * registerTimer(unsigned activityId, const char * name)
     {
         CriticalBlock b(contextCrit);
-        if (activityId && graph)
-        {
-            IRoxieServerActivity *act = graph->queryActivity(activityId);
-            if (act)
-                return act->registerTimer(activityId, name);
-        }
         ISectionTimer *timer = functionTimers.getValue(name);
         if (!timer)
         {

+ 1 - 1
roxie/ccd/ccdquery.cpp

@@ -34,7 +34,7 @@ void ActivityArray::append(IActivityFactory &cur)
     activities.append(cur);
 }
 
-unsigned ActivityArray::findActivityIndex(unsigned id)
+unsigned ActivityArray::findActivityIndex(unsigned id) const
 {
     unsigned *ret = hash.getValue(id);
     if (ret)

+ 1 - 2
roxie/ccd/ccdquery.hpp

@@ -67,7 +67,6 @@ interface IActivityGraph : extends IInterface
     virtual IRoxieServerChildGraph * queryLoopGraph() = 0;
     virtual IRoxieServerChildGraph * createGraphLoopInstance(IRoxieSlaveContext *ctx, unsigned loopCounter, unsigned parentExtractSize, const byte * parentExtract, const IRoxieContextLogger &logctx) = 0;
     virtual const char *queryName() const = 0;
-    virtual IRoxieServerActivity *queryActivity(unsigned _activityId) = 0;
 };
 
 interface IRoxiePackage;
@@ -199,7 +198,7 @@ public:
         libraryGraphId = 0;
     }
 
-    unsigned findActivityIndex(unsigned id);
+    unsigned findActivityIndex(unsigned id) const;
     unsigned recursiveFindActivityIndex(unsigned id);
     inline IActivityFactory &item(unsigned idx) const { return activities.item(idx); }
     inline IRoxieServerActivityFactory &serverItem(unsigned idx) const { return (IRoxieServerActivityFactory &) activities.item(idx); }

+ 25 - 30
roxie/ccd/ccdserver.cpp

@@ -921,6 +921,21 @@ protected:
 
     IHThorArg &basehelper;
     IRoxieSlaveContext *ctx;
+    class InterceptRegisterTimer : public IndirectCodeContext
+    {
+    public:
+        InterceptRegisterTimer(CRoxieServerActivity &_activity) : activity(_activity) {}
+        virtual ISectionTimer *registerTimer(unsigned activityId, const char * name)
+        {
+            if (activityId == activity.queryId())
+                return activity.registerTimer(activityId, name);
+            else
+                return ctx->registerTimer(activityId, name);
+        }
+    private:
+        CRoxieServerActivity &activity;
+    } interceptedCtx;
+
     const IRoxieServerActivityFactory *factory;
     IRoxieServerActivityCopyArray dependencies;
     IntArray dependencyIndexes;
@@ -951,6 +966,7 @@ public:
 
     CRoxieServerActivity(IRoxieSlaveContext *_ctx, const IRoxieServerActivityFactory *_factory, IProbeManager *_probeManager)
         : ctx(_ctx),
+          interceptedCtx(*this),
           factory(_factory),
           basehelper(_factory->getHelper()),
           activityId(_factory->queryId()),
@@ -972,7 +988,10 @@ public:
         aborted = false;
     }
     
-    CRoxieServerActivity(IRoxieSlaveContext *_ctx, IHThorArg & _helper) : ctx(_ctx), factory(NULL), basehelper(_helper), stats(allStatistics)
+    CRoxieServerActivity(IRoxieSlaveContext *_ctx, IHThorArg & _helper)
+    : ctx(_ctx),
+      interceptedCtx(*this),
+      factory(NULL), basehelper(_helper), stats(allStatistics)
     {
         activityId = 0;
         input = NULL;
@@ -1038,7 +1057,7 @@ public:
     {
         stats.deserializeMerge(buf);
     }
-    virtual ISectionTimer *registerTimer(unsigned activityId, const char * name)
+    virtual ISectionTimer *registerTimer(unsigned _activityId, const char * name)
     {
         CriticalBlock b(statscrit); // reuse statscrit to protect functionTimers - it will not be held concurrently
         ISectionTimer *timer = functionTimers.getValue(name);
@@ -1050,19 +1069,6 @@ public:
         }
         return timer;
     }
-    virtual IRoxieServerActivity * queryChildActivity(unsigned activityId)
-    {
-        ForEachItemIn(i, childGraphs)
-        {
-            IRoxieServerActivity * activity = childGraphs.item(i).queryActivity(activityId);
-            if (activity)
-                return activity;
-        }
-#ifdef _DEBUG
-        throwUnexpectedX("Unable to map child activity id to an activity");
-#endif
-        return nullptr;
-    }
     void mergeStrandStats(unsigned strandProcessed, const ActivityTimeAccumulator & strandCycles, const CRuntimeStatisticCollection & strandStats)
     {
         CriticalBlock cb(statscrit);
@@ -1309,7 +1315,8 @@ public:
         if (createPending)
         {
             createPending = false;
-            basehelper.onCreate(ctx->queryCodeContext(), colocalParent, NULL);
+            interceptedCtx.set(ctx->queryCodeContext());
+            basehelper.onCreate(&interceptedCtx, colocalParent, NULL);
         }
     }
 
@@ -26824,7 +26831,7 @@ protected:
     StringAttr graphName;
     Owned<CGraphResults> results;
     CGraphResults graphLoopResults;
-    ActivityArray & graphDefinition;
+    const ActivityArray & graphDefinition;
     CriticalSection evaluateCrit;
 
     IProbeManager *probeManager;
@@ -26834,7 +26841,7 @@ protected:
 public:
     IMPLEMENT_IINTERFACE;
 
-    CActivityGraph(IRoxieSlaveContext *_ctx, const char *_graphName, unsigned _id, ActivityArray &x, IProbeManager *_probeManager, const IRoxieContextLogger &_logctx)
+    CActivityGraph(IRoxieSlaveContext *_ctx, const char *_graphName, unsigned _id, const ActivityArray &x, IProbeManager *_probeManager, const IRoxieContextLogger &_logctx)
         : probeManager(_probeManager), graphDefinition(x), graphName(_graphName), graphSlaveContext(_ctx, _logctx)
     {
         id = x.getLibraryGraphId();
@@ -26856,18 +26863,6 @@ public:
         return graphName.get();
     }
 
-    virtual IRoxieServerActivity *queryActivity(unsigned _activityId)
-    {
-        unsigned idx = graphDefinition.recursiveFindActivityIndex(_activityId);
-        if (idx==NotFound)
-            return nullptr;
-        assertex(activities.isItem(idx));
-        IRoxieServerActivity *activity = &activities.item(idx);
-        if (activity->queryId() == _activityId)
-            return activity;
-        return activity->queryChildActivity(_activityId);
-    }
-
     void createGraph(IRoxieSlaveContext *_ctx)
     {
         if (graphDefinition.isMultiInstance())

+ 0 - 1
roxie/ccd/ccdserver.hpp

@@ -190,7 +190,6 @@ interface IRoxieServerActivity : extends IActivityBase
     virtual const IRoxieContextLogger &queryLogCtx() const = 0;
     virtual void mergeStats(MemoryBuffer &stats) = 0;
     virtual ISectionTimer * registerTimer(unsigned activityId, const char * name) = 0;
-    virtual IRoxieServerActivity * queryChildActivity(unsigned activityId) = 0;
     virtual IEngineRowAllocator * createRowAllocator(IOutputMetaData * metadata) = 0;
 };
 

+ 3 - 3
rtl/include/eclhelper.hpp

@@ -628,15 +628,15 @@ interface ICodeContext : public IResourceContext
     // Graphs, child queries etc
 
     virtual void executeGraph(const char * graphName, bool realThor, size32_t parentExtractSize, const void * parentExtract) = 0;
-    virtual unsigned getGraphLoopCounter() const { return 0; }
+    virtual unsigned getGraphLoopCounter() const = 0;
     virtual IThorChildGraph * resolveChildQuery(__int64 activityId, IHThorArg * colocal) = 0;
-    virtual IEclGraphResults * resolveLocalQuery(__int64 activityId) { return NULL; }
+    virtual IEclGraphResults * resolveLocalQuery(__int64 activityId) = 0;
 
     // Logging etc
 
     virtual unsigned logString(const char *text) const = 0;
     virtual const IContextLogger &queryContextLogger() const = 0;
-    virtual IDebuggableContext *queryDebugContext() const { return NULL; }
+    virtual IDebuggableContext *queryDebugContext() const = 0;
 
     // Memory management
 

+ 1 - 1
testing/regress/ecl/embedpy3.ecl

@@ -23,7 +23,7 @@ IMPORT Python3;
 
 Python3.Language.syntaxcheck('1+2');
 
-integer add1(integer val) := EMBED(Python3)
+integer add1(integer val) := EMBED(Python3 : TIME)
 val+1
 ENDEMBED;
 

+ 1 - 0
thorlcr/graph/thgraph.hpp

@@ -540,6 +540,7 @@ class graph_decl CGraphBase : public CGraphStub, implements IEclGraphResults
 
         virtual void getRowXML(size32_t & lenResult, char * & result, IOutputMetaData & info, const void * row, unsigned flags) { convertRowToXML(lenResult, result, info, row, flags); }
         virtual void getRowJSON(size32_t & lenResult, char * & result, IOutputMetaData & info, const void * row, unsigned flags) { convertRowToJSON(lenResult, result, info, row, flags); }
+        virtual IDebuggableContext *queryDebugContext() const override { return ctx->queryDebugContext(); }
         virtual unsigned getGraphLoopCounter() const
         {
             return containerGraph->queryLoopCounter();           // only called if value is valid

+ 2 - 0
thorlcr/graph/thgraphmaster.cpp

@@ -833,6 +833,8 @@ public:
     }
 
 // ICodeContext
+    virtual unsigned getGraphLoopCounter() const override { return 0; }
+    virtual IDebuggableContext *queryDebugContext() const override { return nullptr; }
     virtual void setResultBool(const char *name, unsigned sequence, bool result)
     {
         WorkunitUpdate w(&workunit->lock());

+ 2 - 0
thorlcr/graph/thgraphslave.cpp

@@ -1372,6 +1372,8 @@ public:
         e->setSeverity((ErrorSeverity)severity);
         jobChannel.fireException(e);
     }
+    virtual unsigned getGraphLoopCounter() const override { return 0; }
+    virtual IDebuggableContext *queryDebugContext() const override { return nullptr; }
     virtual unsigned getNodes() { return jobChannel.queryJob().querySlaves(); }
     virtual unsigned getNodeNum() { return jobChannel.queryMyRank()-1; }
     virtual char *getFilePart(const char *logicalName, bool create=false)