From 0a855add8a7273a940adcb52a4ca208556b88564 Mon Sep 17 00:00:00 2001 From: Shamser Ahmed Date: Thu, 11 Jul 2024 13:46:56 +0100 Subject: [PATCH] HPCC-32138 Changes following review Signed-off-by: Shamser Ahmed --- system/jlib/jstats.h | 9 ++++++--- thorlcr/activities/hashdistrib/thhashdistribslave.cpp | 4 ++-- thorlcr/activities/nsplitter/thnsplitterslave.cpp | 2 +- thorlcr/thorutil/thbuf.cpp | 11 ++++------- thorlcr/thorutil/thormisc.cpp | 2 +- thorlcr/thorutil/thormisc.hpp | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/system/jlib/jstats.h b/system/jlib/jstats.h index 300c5801ea7..e3c17798b2c 100644 --- a/system/jlib/jstats.h +++ b/system/jlib/jstats.h @@ -875,8 +875,11 @@ void setStat(CRuntimeStatisticCollection & stats, INTERFACE * source, StatisticK template void setStat(CRuntimeStatisticCollection & stats, const Shared & source, StatisticKind kind) { setStat(stats, source.get(), kind); } + +typedef std::map StatKindMap; + template -void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, const StatisticsMapping & mapping, const std::map & remaps) +void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, const StatisticsMapping & mapping, const StatKindMap & remaps) { if (!source) return; @@ -895,13 +898,13 @@ void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, } template -void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, const std::map & remaps) +void mergeRemappedStats(CRuntimeStatisticCollection & stats, INTERFACE * source, const StatKindMap & remaps) { mergeRemappedStats(stats, source, stats.queryMapping(), remaps); } template -void mergeRemappedStats(CRuntimeStatisticCollection & stats, const Shared & source, const std::map & remaps) +void mergeRemappedStats(CRuntimeStatisticCollection & stats, const Shared & source, const StatKindMap & remaps) { mergeRemappedStats(stats, source.get(), stats.queryMapping(), remaps); } diff --git a/thorlcr/activities/hashdistrib/thhashdistribslave.cpp b/thorlcr/activities/hashdistrib/thhashdistribslave.cpp index ce09ded235c..da214c0f16a 100644 --- a/thorlcr/activities/hashdistrib/thhashdistribslave.cpp +++ b/thorlcr/activities/hashdistrib/thhashdistribslave.cpp @@ -2760,8 +2760,8 @@ class CSpill : implements IRowWriter, public CSimpleInterface ::Release(writer); writer = NULL; spillFileIO->flush(); - mergeRemappedStats(stats, spillFileIO, diskToSpillStatsMap); - stats.setStatistic(StNumSpills, 1); + mergeRemappedStats(stats, spillFileIO, diskToTempStatsMap); + stats.addStatistic(StNumSpills, 1); spillFile->noteSize(spillFileIO->getStatistic(StSizeDiskWrite)); spillFileIO.clear(); } diff --git a/thorlcr/activities/nsplitter/thnsplitterslave.cpp b/thorlcr/activities/nsplitter/thnsplitterslave.cpp index c95096b1835..488f1a582e4 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) - mergeRemappedStats(activeStats, sharedRowStream, diskToSpillStatsMap); + mergeRemappedStats(activeStats, sharedRowStream, diskToTempStatsMap); } // ISharedSmartBufferCallback impl. virtual void paged() { pagedOut = true; } diff --git a/thorlcr/thorutil/thbuf.cpp b/thorlcr/thorutil/thbuf.cpp index c7c684c291a..fd7db431143 100644 --- a/thorlcr/thorutil/thbuf.cpp +++ b/thorlcr/thorutil/thbuf.cpp @@ -2482,11 +2482,11 @@ class CSharedFullSpillingWriteAhead : public CInterfaceOfflush(); tempFileOwner->noteSize(iFileIO->getStatistic(StSizeDiskWrite)); ::mergeStats(inactiveStats, iFileIO); iFileIO.clear(); - outputStream.clear(); } } void createOutputStream() @@ -2497,6 +2497,7 @@ class CSharedFullSpillingWriteAhead : public CInterfaceOf(res)); iFileIO.setown(std::get<1>(res)); totalInputRowsRead = inMemTotalRows; + inactiveStats.addStatistic(StNumSpills, 1); } void writeRowsFromInput() { @@ -2538,6 +2539,7 @@ class CSharedFullSpillingWriteAhead : public CInterfaceOfflush(); totalInputRowsRead.fetch_add(newRowsWritten); tempFileOwner->noteSize(iFileIO->getStatistic(StSizeDiskWrite)); + ::mergeStats(inactiveStats, iFileIO); // JCSMORE - could track size written, and start new file at this point (e.g. every 100MB), // and track their starting points (by row #) in a vector // We could then tell if/when the readers catch up, and remove consumed files as they do. @@ -2715,12 +2717,7 @@ class CSharedFullSpillingWriteAhead : public CInterfaceOfgetStatistic(kind); - return v; + return inactiveStats.getStatisticValue(kind); } }; diff --git a/thorlcr/thorutil/thormisc.cpp b/thorlcr/thorutil/thormisc.cpp index 84aec4bc6b7..32ab87ebf26 100644 --- a/thorlcr/thorutil/thormisc.cpp +++ b/thorlcr/thorutil/thormisc.cpp @@ -99,7 +99,7 @@ const StatisticsMapping hashDistribActivityStatistics({StNumLocalRows, StNumRemo const StatisticsMapping nsplitterActivityStatistics({}, spillStatistics, basicActivityStatistics); const StatisticsMapping spillingWriteAheadStatistics({}, spillStatistics); -const std::map diskToSpillStatsMap +const StatKindMap diskToTempStatsMap ={ {StSizeDiskWrite, StSizeSpillFile}, {StTimeDiskWriteIO, StTimeSpillElapsed} }; diff --git a/thorlcr/thorutil/thormisc.hpp b/thorlcr/thorutil/thormisc.hpp index 4c0c2aa349d..a5db68707e5 100644 --- a/thorlcr/thorutil/thormisc.hpp +++ b/thorlcr/thorutil/thormisc.hpp @@ -171,7 +171,7 @@ 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; +extern graph_decl const std::map diskToTempStatsMap; class BooleanOnOff {