From 776416d2d34547589e7a78e978b00d4c1841f038 Mon Sep 17 00:00:00 2001 From: Shamser Ahmed Date: Thu, 27 Jun 2024 09:49:59 +0100 Subject: [PATCH] HPCC-32138 Generic remapping merge function to remap disk stats to spill stats Signed-off-by: Shamser Ahmed --- system/jlib/jstats.h | 32 ++++++++++++++ .../hashdistrib/thhashdistribslave.cpp | 21 ++------- .../activities/nsplitter/thnsplitterslave.cpp | 2 +- thorlcr/thorutil/thbuf.cpp | 43 +++---------------- thorlcr/thorutil/thormisc.cpp | 5 +++ thorlcr/thorutil/thormisc.hpp | 3 ++ 6 files changed, 51 insertions(+), 55 deletions(-) diff --git a/system/jlib/jstats.h b/system/jlib/jstats.h index 253d640c94d..300c5801ea7 100644 --- a/system/jlib/jstats.h +++ b/system/jlib/jstats.h @@ -23,6 +23,7 @@ #include "jmutex.hpp" #include #include +#include #include "jstatcodes.h" @@ -874,6 +875,37 @@ void setStat(CRuntimeStatisticCollection & stats, INTERFACE * source, StatisticK template void setStat(CRuntimeStatisticCollection & stats, const Shared & source, StatisticKind kind) { setStat(stats, source.get(), kind); } +template +void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, const StatisticsMapping & mapping, const std::map & remaps) +{ + if (!source) + return; + unsigned max = mapping.numStatistics(); + for (unsigned i=0; i < max; i++) + { + StatisticKind kind = mapping.getKind(i); + if (remaps.find(kind) == remaps.end()) + stats.mergeStatistic(kind, source->getStatistic(kind)); + } + for (auto remap: remaps) + { + if (mapping.hasKind(remap.second)) + stats.mergeStatistic(remap.second, source->getStatistic(remap.first)); + } +} + +template +void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, const std::map & remaps) +{ + mergeRemappedStats(stats, source, stats.queryMapping(), remaps); +} + +template +void mergeRemappedStats(CRuntimeStatisticCollection & stats, const Shared & source, const std::map & remaps) +{ + mergeRemappedStats(stats, source.get(), stats.queryMapping(), remaps); +} + //--------------------------------------------------------------------------------------------------------------------- //A class for minimizing the overhead of collecting timestamps. diff --git a/thorlcr/activities/hashdistrib/thhashdistribslave.cpp b/thorlcr/activities/hashdistrib/thhashdistribslave.cpp index 61a49ced9a9..ce09ded235c 100644 --- a/thorlcr/activities/hashdistrib/thhashdistribslave.cpp +++ b/thorlcr/activities/hashdistrib/thhashdistribslave.cpp @@ -2760,26 +2760,11 @@ class CSpill : implements IRowWriter, public CSimpleInterface ::Release(writer); writer = NULL; spillFileIO->flush(); - mergeStats(stats, this); - spillFile->noteSize(getStatistic(StSizeSpillFile)); + mergeRemappedStats(stats, spillFileIO, diskToSpillStatsMap); + stats.setStatistic(StNumSpills, 1); + spillFile->noteSize(spillFileIO->getStatistic(StSizeDiskWrite)); spillFileIO.clear(); } - inline __int64 getStatistic(StatisticKind kind) const - { - switch (kind) - { - case StSizeSpillFile: - return spillFileIO->getStatistic(StSizeDiskWrite); - case StTimeSortElapsed: - return spillFileIO->getStatistic(StTimeDiskWriteIO); - case StSizeDiskWrite: - return 0; // Return file size as StSizeSpillFile kind. To avoid confusion, StSizeDiskWrite will not be returned - case StNumSpills: - return 1; - default: - return spillFileIO->getStatistic(kind); - } - } // IRowWriter virtual void putRow(const void *row) override { diff --git a/thorlcr/activities/nsplitter/thnsplitterslave.cpp b/thorlcr/activities/nsplitter/thnsplitterslave.cpp index ca9ccd0d6df..c95096b1835 100644 --- a/thorlcr/activities/nsplitter/thnsplitterslave.cpp +++ b/thorlcr/activities/nsplitter/thnsplitterslave.cpp @@ -405,7 +405,7 @@ class NSplitterSlaveActivity : public CSlaveActivity, implements ISharedSmartBuf { PARENT::gatherActiveStats(activeStats); if (sharedRowStream) - ::mergeStats(activeStats, sharedRowStream); + mergeRemappedStats(activeStats, sharedRowStream, diskToSpillStatsMap); } // ISharedSmartBufferCallback impl. virtual void paged() { pagedOut = true; } diff --git a/thorlcr/thorutil/thbuf.cpp b/thorlcr/thorutil/thbuf.cpp index 5ab144e8ee9..c7c684c291a 100644 --- a/thorlcr/thorutil/thbuf.cpp +++ b/thorlcr/thorutil/thbuf.cpp @@ -2151,21 +2151,9 @@ class CSharedWriteAheadDisk : public CSharedWriteAheadBase } virtual unsigned __int64 getStatistic(StatisticKind kind) const override { - switch (kind) - { - case StSizeSpillFile: - return tempFileIO->getStatistic(StSizeDiskWrite); - case StCycleDiskWriteIOCycles: - case StTimeDiskWriteIO: - case StSizeDiskWrite: - return 0; - case StNumSpills: - return 1; - case StTimeSpillElapsed: - return tempFileIO->getStatistic(StCycleDiskWriteIOCycles); - default: - return tempFileIO->getStatistic(kind); - } + if (kind==StNumSpills) + return 1; + return tempFileIO->getStatistic(kind); } }; @@ -2727,28 +2715,11 @@ class CSharedFullSpillingWriteAhead : public CInterfaceOfgetStatistic(useKind); - v += inactiveStats.getStatisticValue(useKind); + v = iFileIO->getStatistic(kind); return v; } }; diff --git a/thorlcr/thorutil/thormisc.cpp b/thorlcr/thorutil/thormisc.cpp index 09b3fc233af..84aec4bc6b7 100644 --- a/thorlcr/thorutil/thormisc.cpp +++ b/thorlcr/thorutil/thormisc.cpp @@ -99,6 +99,11 @@ const StatisticsMapping hashDistribActivityStatistics({StNumLocalRows, StNumRemo const StatisticsMapping nsplitterActivityStatistics({}, spillStatistics, basicActivityStatistics); const StatisticsMapping spillingWriteAheadStatistics({}, spillStatistics); +const std::map diskToSpillStatsMap +={ {StSizeDiskWrite, StSizeSpillFile}, + {StTimeDiskWriteIO, StTimeSpillElapsed} + }; + MODULE_INIT(INIT_PRIORITY_STANDARD) { ClusterMPAllocator.setown(createMPtagRangeAllocator(MPTAG_THORGLOBAL_BASE,MPTAG_THORGLOBAL_COUNT)); diff --git a/thorlcr/thorutil/thormisc.hpp b/thorlcr/thorutil/thormisc.hpp index cf0b92bb4b0..4c0c2aa349d 100644 --- a/thorlcr/thorutil/thormisc.hpp +++ b/thorlcr/thorutil/thormisc.hpp @@ -170,6 +170,9 @@ extern graph_decl const StatisticsMapping hashDistribActivityStatistics; extern graph_decl const StatisticsMapping nsplitterActivityStatistics; extern graph_decl const StatisticsMapping spillingWriteAheadStatistics; +// Maps disk related stats to spill stats +extern graph_decl const std::map diskToSpillStatsMap; + class BooleanOnOff { bool &tf;