From de4785f07dc5a27a12719b7df7f87c0bd502437f Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 11:43:45 +0100 Subject: [PATCH 01/21] refactor unit test to reduce redundancy --- .../unittest/CEventRateDataGathererTest.cc | 177 +++++++++++------- 1 file changed, 107 insertions(+), 70 deletions(-) diff --git a/lib/model/unittest/CEventRateDataGathererTest.cc b/lib/model/unittest/CEventRateDataGathererTest.cc index d62ef85a6..9dc4895ee 100644 --- a/lib/model/unittest/CEventRateDataGathererTest.cc +++ b/lib/model/unittest/CEventRateDataGathererTest.cc @@ -63,6 +63,61 @@ namespace { const CSearchKey key; const std::string EMPTY_STRING; +class CDataGathererBuilder { +public: + CDataGathererBuilder(const TFeatureVec& features, const SModelParams& params, const CSearchKey& searchKey, + const core_t::TTime startTime + ): m_Features(features), m_Params(params), m_StartTime(startTime), m_SearchKey(searchKey) {} + + CDataGatherer build() const { + return {m_GathererType, m_SummaryMode, m_Params, m_SummaryCountFieldName, + m_PartitionFieldValue, m_PersonFieldName, m_AttributeFieldName, + m_ValueFieldName, m_InfluenceFieldNames, m_SearchKey, m_Features, + m_StartTime, m_SampleCountOverride}; + } + + CDataGathererBuilder& personFieldName(const std::string& personFieldName) { + m_PersonFieldName = personFieldName; + return *this; + } + + CDataGathererBuilder& valueFieldName(const std::string& valueFieldName) { + m_ValueFieldName = valueFieldName; + return *this; + } + + CDataGathererBuilder& influenceFieldNames(const TStrVec& influenceFieldName) { + m_InfluenceFieldNames = influenceFieldName; + return *this; + } + + CDataGathererBuilder& attributeFieldName(const std::string& attributeFieldName) { + m_AttributeFieldName = attributeFieldName; + return *this; + } + + CDataGathererBuilder& gathererType(model_t::EAnalysisCategory gathererType) { + m_GathererType = gathererType; + return *this; + } + +private: + const TFeatureVec& m_Features; + const SModelParams& m_Params; + core_t::TTime m_StartTime; + const CSearchKey& m_SearchKey; + model_t::EAnalysisCategory m_GathererType{model_t::E_EventRate}; + model_t::ESummaryMode m_SummaryMode{model_t::E_None}; + std::string m_SummaryCountFieldName{EMPTY_STRING}; + std::string m_PartitionFieldValue{EMPTY_STRING}; + std::string m_PersonFieldName{EMPTY_STRING}; + std::string m_AttributeFieldName{EMPTY_STRING}; + std::string m_ValueFieldName{EMPTY_STRING}; + TStrVec m_InfluenceFieldNames; + int m_SampleCountOverride{0}; + +}; + std::size_t addPerson(CDataGatherer& gatherer, CResourceMonitor& resourceMonitor, const std::string& p, @@ -136,7 +191,7 @@ void addArrival(CDataGatherer& gatherer, CDataGatherer::TStrCPtrVec fieldValues; fieldValues.push_back(&person); - for (const auto& influencer : influencers) { + for (const auto & influencer : influencers) { fieldValues.push_back(&influencer); } @@ -166,8 +221,7 @@ void testInfluencerPerFeature(const model_t::EFeature feature, features.push_back(feature); TStrVec influencerFieldNames; influencerFieldNames.emplace_back("IF1"); - CDataGatherer gatherer = CDataGathererBuilder(model_t::E_EventRate, features, - params, key, startTime) + CDataGatherer gatherer = CDataGathererBuilder(features, params, key, startTime) .influenceFieldNames(influencerFieldNames) .valueFieldName(valueField) .build(); @@ -494,9 +548,9 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeries, CTestFixture) { 10000 // sentinel }; - std::array const expectedPersonCounts{ - std::string("[(0, 6)]"), std::string("[(0, 3)]"), std::string("[(0, 2)]"), - std::string("[(0, 0)]"), std::string("[(0, 3)]")}; + std::array const expectedPersonCounts{std::string("[(0, 6)]"), std::string("[(0, 3)]"), + std::string("[(0, 2)]"), std::string("[(0, 0)]"), + std::string("[(0, 3)]")}; std::array const expectedPersonNonZeroCounts{ std::string("[(0, 6)]"), std::string("[(0, 3)]"), @@ -638,7 +692,7 @@ BOOST_FIXTURE_TEST_CASE(testMultipleSeries, CTestFixture) { constexpr core_t::TTime startTime = 0; constexpr core_t::TTime bucketLength = 600; - const std::vector data1 = { + const std::vector data1 = { 1, 15, 180, 190, 400, 550, // bucket 1 600, 799, @@ -744,7 +798,7 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { BOOST_REQUIRE_EQUAL(4, addPerson(expectedGatherer, m_ResourceMonitor, "p7")); BOOST_REQUIRE_EQUAL(5, addPerson(expectedGatherer, m_ResourceMonitor, "p8")); - constexpr std::array expectedCounts = {5, 2, 0, + constexpr std::array expectedCounts = {5, 2, 0, 5, 7, 10}; for (std::size_t i = 0; i < std::size(expectedCounts); ++i) { for (core_t::TTime time = 0; time < expectedCounts[i]; ++time) { @@ -821,7 +875,7 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeriesOutOfOrderFinalResult, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - const std::array data = { + const std::array data = { 1, 180, 1200, 190, 400, 600, // bucket 1, 2 & 3 550, 799, 1199, @@ -834,6 +888,7 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeriesOutOfOrderFinalResult, CTestFixture) { }; const std::array expectedPersonCounts = { + constexpr std::array expectedPersonCounts = { std::string("[(0, 6)]"), std::string("[(0, 3)]"), std::string("[(0, 2)]"), std::string("[(0, 0)]"), std::string("[(0, 3)]")}; @@ -968,7 +1023,7 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeriesOutOfOrderInterimResult, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - constexpr std::array data = { + constexpr std::array data = { 1, 1200, 600, // bucket 1, 3 & 2 1199, @@ -1133,9 +1188,9 @@ BOOST_FIXTURE_TEST_CASE(testMultipleSeriesOutOfOrderFinalResult, CTestFixture) { .build(); testGathererMultipleSeries( - STestTimes{.s_StartTime = startTime, .s_BucketLength = bucketLength}, - STestData{.data1 = data1, .data2 = data2}, expectedPersonCounts, - params, latencyTime, gatherer, m_ResourceMonitor); + STestTimes{.s_StartTime= startTime, .s_BucketLength= bucketLength}, + STestData{.data1 =data1, .data2=data2}, expectedPersonCounts, + params, latencyTime, gatherer, m_ResourceMonitor); } { @@ -1156,7 +1211,7 @@ BOOST_FIXTURE_TEST_CASE(testArrivalBeforeLatencyWindowIsIgnored, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - constexpr std::array data = { + constexpr std::array data = { 1800, // Bucket 4, thus bucket 1's values are already out of latency window 1 // Bucket 1 }; @@ -1197,7 +1252,7 @@ BOOST_FIXTURE_TEST_CASE(testResetBucketGivenSingleSeries, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - constexpr std::array data = { + constexpr std::array data = { 100, 300, // Bucket 1 600, 800, @@ -1329,7 +1384,7 @@ BOOST_FIXTURE_TEST_CASE(testResetBucketGivenBucketNotAvailable, CTestFixture) { } BOOST_FIXTURE_TEST_CASE(testInfluencerBucketStatistics, CTestFixture) { - constexpr std::array data = { + constexpr std::array data = { 1, 15, 180, 190, 400, 550, // bucket 1 600, 799, @@ -1406,7 +1461,7 @@ class CDistinctStringsTestFixture : public CTestFixture { // Helper that creates a SEventRateFeatureData object, populates it using the distinct count method, // and checks that its print() output matches the expected value. static void verifyDistinctCountFeature(const CUniqueStringFeatureData& data, - const std::string& expected) { + const std::string& expected) { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); BOOST_REQUIRE_EQUAL(expected, featureData.print()); @@ -1414,7 +1469,7 @@ class CDistinctStringsTestFixture : public CTestFixture { // Similar helper for the info-content feature data. static void verifyInfoContentFeature(const CUniqueStringFeatureData& data, - const std::string& expected) { + const std::string& expected) { SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); BOOST_REQUIRE_EQUAL(expected, featureData.print()); @@ -1457,7 +1512,7 @@ class CDistinctStringsTestFixture : public CTestFixture { data.populateDistinctCountFeatureData(featureData); BOOST_REQUIRE_EQUAL(std::max(static_cast(3), static_cast(i)), - featureData.s_Count); + featureData.s_Count); } } @@ -1488,9 +1543,8 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); - std::sort(featureData.s_InfluenceValues[0].begin(), - featureData.s_InfluenceValues[0].end(), - maths::common::COrderings::SFirstLess()); + std::ranges::sort(featureData.s_InfluenceValues[0], + maths::common::COrderings::SFirstLess()); BOOST_REQUIRE_EQUAL(std::string("3, [[(inf1, ([2], 1)), (inf2, ([2], 1)), (inf3, ([1], 1))]]"), featureData.print()); } @@ -1523,9 +1577,8 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { - std::sort(featureData.s_InfluenceValues[i].begin(), - featureData.s_InfluenceValues[i].end(), - maths::common::COrderings::SFirstLess()); + std::ranges::sort(featureData.s_InfluenceValues[i], + maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("3, [[(inf1, ([2], 1)), (inf2, ([2], 1))], [(inf_v2, ([1], 1)), (inf_v3, ([2], 1))]]"), featureData.print()); @@ -1551,12 +1604,9 @@ class CDistinctStringsTestFixture : public CTestFixture { data.insert(ss.str(), influencers); SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); - BOOST_TEST_REQUIRE((featureData.s_Count - 12) >= - std::max(static_cast(3), + BOOST_TEST_REQUIRE((featureData.s_Count - 12) >= std::max(static_cast(3), static_cast(i))); - BOOST_TEST_REQUIRE( - (featureData.s_Count - 12) <= - std::max(static_cast(3), static_cast(i)) * 3); + BOOST_TEST_REQUIRE((featureData.s_Count - 12) <= std::max(static_cast(3), static_cast(i)) * 3); } } @@ -1620,8 +1670,7 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { - std::sort(featureData.s_InfluenceValues[i].begin(), - featureData.s_InfluenceValues[i].end(), + std::ranges::sort(featureData.s_InfluenceValues[i], maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("18, [[(inf1, ([16], 1)), (inf2, ([16], 1))], [(inf_v2, ([12], 1)), (inf_v3, ([16], 1))]]"), @@ -1661,14 +1710,10 @@ class CDistinctStringsTestFixture : public CTestFixture { testPersistence(params, gatherer, model_t::E_EventRate); // Add data (some out-of-order) for distinct strings. - addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", - "stringOne", "inf1"); - addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", - "stringTwo", "inf2"); - addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", - "stringThree", "inf3"); - addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", - "stringFour", "inf1"); + addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", "stringOne", "inf1"); + addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", "stringTwo", "inf2"); + addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", "stringThree", "inf3"); + addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", "stringFour", "inf1"); addArrival(gatherer, m_ResourceMonitor, time, "p", "stringFive", "inf2"); addArrival(gatherer, m_ResourceMonitor, time, "p", "stringSix", "inf3"); testPersistence(params, gatherer, model_t::E_EventRate); @@ -1760,7 +1805,9 @@ class CDiurnalTestFixture : public CTestFixture { // - TFeatureSizeFeatureDataPrVecPrVec for tests by person, // - TFeatureSizeSizePrFeatureDataPrVecPrVec for tests over person. template - void verifyFeatureData(const CDataGatherer& gatherer, core_t::TTime time, std::uint64_t expectedCount) { + void verifyFeatureData(const CDataGatherer& gatherer, + core_t::TTime time, + std::uint64_t expectedCount) { FeatureDataT featureData; gatherer.featureData(time, BUCKET_LENGTH, featureData); BOOST_REQUIRE_EQUAL(1, featureData.size()); @@ -1803,7 +1850,8 @@ class CDiurnalTestFixture : public CTestFixture { // Arrival 6: time + 300, expected additional count of 150. addArrivalHelper(gatherer, time + 300, useAttribute); - verifyFeatureData(gatherer, time, computeExpected(time, 150, isDay)); + verifyFeatureData(gatherer, time, + computeExpected(time, 150, isDay)); // Check latency: go back two buckets. time -= BUCKET_LENGTH * 2; @@ -1812,13 +1860,12 @@ class CDiurnalTestFixture : public CTestFixture { time += BUCKET_LENGTH; addArrivalHelper(gatherer, time + 400, useAttribute); - verifyFeatureData(gatherer, time, computeExpected(time, 200, isDay)); + verifyFeatureData(gatherer, time, + computeExpected(time, 200, isDay)); } // Verify summary information for the gatherer. - static void verifyGathererSummary(const CDataGatherer& gatherer, - bool isPopulation, - bool useAttribute) { + static void verifyGathererSummary(const CDataGatherer& gatherer, bool isPopulation, bool useAttribute) { if (isPopulation) { if (useAttribute) { BOOST_REQUIRE_EQUAL(1, gatherer.numberActivePeople()); @@ -1838,12 +1885,10 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_day by person LOG_DEBUG(<< "Testing time_of_day by person"); SModelParams const params = createParams(); - TFeatureVec const features{model_t::E_IndividualTimeOfDayByBucketAndPerson}; + TFeatureVec const features{ model_t::E_IndividualTimeOfDayByBucketAndPerson }; CDataGatherer gatherer = createGatherer(features, params, START_TIME, false); - verifyGathererFeatures(gatherer, features, - model_t::E_IndividualTimeOfDayByBucketAndPerson, false); - runTestSequence(gatherer, /*isDay=*/true, - /*useAttribute=*/false); + verifyGathererFeatures(gatherer, features, model_t::E_IndividualTimeOfDayByBucketAndPerson, false); + runTestSequence(gatherer, /*isDay=*/true, /*useAttribute=*/false); verifyGathererSummary(gatherer, false, false); testPersistence(params, gatherer, model_t::E_EventRate); } @@ -1851,12 +1896,10 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_week by person LOG_DEBUG(<< "Testing time_of_week by person"); SModelParams const params = createParams(); - TFeatureVec const features{model_t::E_IndividualTimeOfWeekByBucketAndPerson}; + TFeatureVec const features{ model_t::E_IndividualTimeOfWeekByBucketAndPerson }; CDataGatherer gatherer = createGatherer(features, params, START_TIME, false); - verifyGathererFeatures(gatherer, features, - model_t::E_IndividualTimeOfWeekByBucketAndPerson, false); - runTestSequence( - gatherer, /*isDay=*/false, /*useAttribute=*/false); + verifyGathererFeatures(gatherer, features, model_t::E_IndividualTimeOfWeekByBucketAndPerson, false); + runTestSequence(gatherer, /*isDay=*/false, /*useAttribute=*/false); verifyGathererSummary(gatherer, false, false); testPersistence(params, gatherer, model_t::E_EventRate); } @@ -1864,13 +1907,10 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_week over person (with attribute) LOG_DEBUG(<< "Testing time_of_week over person"); SModelParams const params = createParams(); - TFeatureVec const features{model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute}; - CDataGatherer gatherer = createGatherer(features, params, START_TIME, - true, /*useAttribute=*/true); - verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute, - true); - runTestSequence( - gatherer, /*isDay=*/false, /*useAttribute=*/true); + TFeatureVec const features{ model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute }; + CDataGatherer gatherer = createGatherer(features, params, START_TIME, true, /*useAttribute=*/true); + verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute, true); + runTestSequence(gatherer, /*isDay=*/false, /*useAttribute=*/true); verifyGathererSummary(gatherer, true, true); testPersistence(params, gatherer, model_t::E_EventRate); } @@ -1878,13 +1918,10 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_day over person (with attribute) LOG_DEBUG(<< "Testing time_of_day over person"); SModelParams const params = createParams(); - TFeatureVec const features{model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute}; - CDataGatherer gatherer = createGatherer(features, params, START_TIME, - true, /*useAttribute=*/true); - verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute, - true); - runTestSequence( - gatherer, /*isDay=*/true, /*useAttribute=*/true); + TFeatureVec const features{ model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute }; + CDataGatherer gatherer = createGatherer(features, params, START_TIME, true, /*useAttribute=*/true); + verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute, true); + runTestSequence(gatherer, /*isDay=*/true, /*useAttribute=*/true); verifyGathererSummary(gatherer, true, true); testPersistence(params, gatherer, model_t::E_EventRate); } From 408e8752b96e371f105988a106f857956c1e2202 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 14:18:00 +0100 Subject: [PATCH 02/21] refactor CMetricDataGathererTest.cc --- .../unittest/CEventRateDataGathererTest.cc | 55 +------------------ lib/model/unittest/CMetricDataGathererTest.cc | 11 ++-- 2 files changed, 5 insertions(+), 61 deletions(-) diff --git a/lib/model/unittest/CEventRateDataGathererTest.cc b/lib/model/unittest/CEventRateDataGathererTest.cc index 9dc4895ee..62d7db275 100644 --- a/lib/model/unittest/CEventRateDataGathererTest.cc +++ b/lib/model/unittest/CEventRateDataGathererTest.cc @@ -63,60 +63,7 @@ namespace { const CSearchKey key; const std::string EMPTY_STRING; -class CDataGathererBuilder { -public: - CDataGathererBuilder(const TFeatureVec& features, const SModelParams& params, const CSearchKey& searchKey, - const core_t::TTime startTime - ): m_Features(features), m_Params(params), m_StartTime(startTime), m_SearchKey(searchKey) {} - - CDataGatherer build() const { - return {m_GathererType, m_SummaryMode, m_Params, m_SummaryCountFieldName, - m_PartitionFieldValue, m_PersonFieldName, m_AttributeFieldName, - m_ValueFieldName, m_InfluenceFieldNames, m_SearchKey, m_Features, - m_StartTime, m_SampleCountOverride}; - } - - CDataGathererBuilder& personFieldName(const std::string& personFieldName) { - m_PersonFieldName = personFieldName; - return *this; - } - - CDataGathererBuilder& valueFieldName(const std::string& valueFieldName) { - m_ValueFieldName = valueFieldName; - return *this; - } - - CDataGathererBuilder& influenceFieldNames(const TStrVec& influenceFieldName) { - m_InfluenceFieldNames = influenceFieldName; - return *this; - } - CDataGathererBuilder& attributeFieldName(const std::string& attributeFieldName) { - m_AttributeFieldName = attributeFieldName; - return *this; - } - - CDataGathererBuilder& gathererType(model_t::EAnalysisCategory gathererType) { - m_GathererType = gathererType; - return *this; - } - -private: - const TFeatureVec& m_Features; - const SModelParams& m_Params; - core_t::TTime m_StartTime; - const CSearchKey& m_SearchKey; - model_t::EAnalysisCategory m_GathererType{model_t::E_EventRate}; - model_t::ESummaryMode m_SummaryMode{model_t::E_None}; - std::string m_SummaryCountFieldName{EMPTY_STRING}; - std::string m_PartitionFieldValue{EMPTY_STRING}; - std::string m_PersonFieldName{EMPTY_STRING}; - std::string m_AttributeFieldName{EMPTY_STRING}; - std::string m_ValueFieldName{EMPTY_STRING}; - TStrVec m_InfluenceFieldNames; - int m_SampleCountOverride{0}; - -}; std::size_t addPerson(CDataGatherer& gatherer, CResourceMonitor& resourceMonitor, @@ -221,7 +168,7 @@ void testInfluencerPerFeature(const model_t::EFeature feature, features.push_back(feature); TStrVec influencerFieldNames; influencerFieldNames.emplace_back("IF1"); - CDataGatherer gatherer = CDataGathererBuilder(features, params, key, startTime) + CDataGatherer gatherer = CDataGathererBuilder(model_t::E_EventRate, features, params, key, startTime) .influenceFieldNames(influencerFieldNames) .valueFieldName(valueField) .build(); diff --git a/lib/model/unittest/CMetricDataGathererTest.cc b/lib/model/unittest/CMetricDataGathererTest.cc index 76feaf054..1dc81d8ae 100644 --- a/lib/model/unittest/CMetricDataGathererTest.cc +++ b/lib/model/unittest/CMetricDataGathererTest.cc @@ -1128,10 +1128,7 @@ BOOST_FIXTURE_TEST_CASE(testResetBucketGivenMultipleSeries, CTestFixture) { features.push_back(model_t::E_IndividualMinByPerson); features.push_back(model_t::E_IndividualMaxByPerson); features.push_back(model_t::E_IndividualSumByBucketAndPerson); - CDataGatherer gatherer = - CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) - .sampleCountOverride(2U) - .build(); + CDataGatherer gatherer = CDataGathererBuilder(model_t::E_Metric,features, params, KEY, startTime).sampleCountOverride(2U).build(); addPerson("p1", gatherer, m_ResourceMonitor); addPerson("p2", gatherer, m_ResourceMonitor); addPerson("p3", gatherer, m_ResourceMonitor); @@ -1604,9 +1601,9 @@ BOOST_FIXTURE_TEST_CASE(testMultivariate, CTestFixture) { { TFeatureVec features; features.push_back(model_t::E_IndividualMeanLatLongByPerson); - CDataGatherer gatherer = CDataGathererBuilder(model_t::E_Metric, features, - params, KEY, startTime) - .build(); + CDataGatherer gatherer(model_t::E_Metric, model_t::E_None, params, + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, + EMPTY_STRING, {}, KEY, features, startTime, 0); BOOST_REQUIRE_EQUAL(0, addPerson("p", gatherer, m_ResourceMonitor)); TTimeDoubleDoubleTupleVecVec buckets; From ab4f7c5b3127eac617db276d5ab2985ad9a0c08c Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 15:14:37 +0100 Subject: [PATCH 03/21] refactor tests to remove direct construction call to CDataGatherer --- lib/model/unittest/CDetectionRuleTest.cc | 35 +++++++++++++++++++ .../CEventRatePopulationDataGathererTest.cc | 8 ++--- .../unittest/CHierarchicalResultsTest.cc | 13 +++---- lib/model/unittest/ModelTestHelpers.h | 12 +++++++ 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/lib/model/unittest/CDetectionRuleTest.cc b/lib/model/unittest/CDetectionRuleTest.cc index 0fdd8af73..19f643cad 100644 --- a/lib/model/unittest/CDetectionRuleTest.cc +++ b/lib/model/unittest/CDetectionRuleTest.cc @@ -794,6 +794,41 @@ BOOST_FIXTURE_TEST_CASE(testRuleActions, CTestFixture) { 0, 0, 100)); } +TMockModelPtr initializeModel(ml::model::CResourceMonitor& resourceMonitor) { + core_t::TTime bucketLength{600}; + model::SModelParams params{bucketLength}; + model::CSearchKey key; + model_t::TFeatureVec features; + // Initialize mock model + model::CAnomalyDetectorModel::TDataGathererPtr gatherer; + + features.assign(1, model_t::E_IndividualSumByBucketAndPerson); + + gatherer = std::make_shared( + model_t::analysisCategory(features[0]), model_t::E_None, params, EMPTY_STRING, + EMPTY_STRING, "p", EMPTY_STRING, EMPTY_STRING, TStrVec{}, key, features, 0, 0); + + std::string person("p1"); + bool addedPerson{false}; + gatherer->addPerson(person, resourceMonitor, addedPerson); + + TMockModelPtr model{new model::CMockModel( + params, gatherer, {/* we don't care about influence */})}; + + maths::time_series::CTimeSeriesDecomposition trend; + maths::common::CNormalMeanPrecConjugate prior{ + maths::common::CNormalMeanPrecConjugate::nonInformativePrior(maths_t::E_ContinuousData)}; + maths::common::CModelParams timeSeriesModelParams{ + bucketLength, 1.0, 0.001, 0.2, 6 * core::constants::HOUR, 24 * core::constants::HOUR}; + std::unique_ptr timeSeriesModel = + std::make_unique( + timeSeriesModelParams, 0, trend, prior); + model::CMockModel::TMathsModelUPtrVec models; + models.emplace_back(std::move(timeSeriesModel)); + model->mockTimeSeriesModels(std::move(models)); + return model; +} + BOOST_FIXTURE_TEST_CASE(testRuleTimeShiftShouldShiftTimeSeriesModelState, CTestFixture) { test::CRandomNumbers rng; diff --git a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc index 702704587..e8e25c752 100644 --- a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc +++ b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc @@ -475,11 +475,11 @@ BOOST_FIXTURE_TEST_CASE(testCompressedLength, CTestFixture) { CDataGatherer::TFeatureVec features; features.push_back(model_t::E_PopulationInfoContentByBucketPersonAndAttribute); - SModelParams const params(bucketLength); + SModelParams constparams(bucketLength); CDataGatherer dataGatherer = CDataGathererBuilder(model_t::E_PopulationEventRate, - features, params, searchKey, startTime) - .valueFieldName("value") - .build(); + features, params, searchKey, startTime) + .valueFieldName("value") + .build(); core_t::TTime time = startTime; for (std::size_t i = 0; i < numberBuckets; ++i, time += bucketLength) { TMessageVec messages; diff --git a/lib/model/unittest/CHierarchicalResultsTest.cc b/lib/model/unittest/CHierarchicalResultsTest.cc index 1491592ec..a6f4c98f9 100644 --- a/lib/model/unittest/CHierarchicalResultsTest.cc +++ b/lib/model/unittest/CHierarchicalResultsTest.cc @@ -1446,12 +1446,13 @@ BOOST_AUTO_TEST_CASE(testWriter) { model::SModelParams const params(modelConfig.bucketLength()); auto interimBucketCorrector = std::make_shared(modelConfig.bucketLength()); - model::CSearchKey const key; - auto dataGatherer = - model::CDataGathererBuilder(model_t::E_EventRate, - {model_t::E_IndividualCountByBucketAndPerson}, - params, key, modelConfig.bucketLength()) - .buildSharedPtr(); + model::CSearchKey key; + model::CAnomalyDetectorModel::TDataGathererPtr dataGatherer( + std::make_shared( + model_t::E_EventRate, model_t::E_None, params, EMPTY_STRING, + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, TStrVec{}, key, + model_t::TFeatureVec{model_t::E_IndividualCountByBucketAndPerson}, + modelConfig.bucketLength(), 0)); model::CEventData dummy; dataGatherer->addArrival(TStrCPtrVec(1, &EMPTY_STRING), dummy, resourceMonitor); dummy.clear(); diff --git a/lib/model/unittest/ModelTestHelpers.h b/lib/model/unittest/ModelTestHelpers.h index ba474c719..d75ca990d 100644 --- a/lib/model/unittest/ModelTestHelpers.h +++ b/lib/model/unittest/ModelTestHelpers.h @@ -124,6 +124,18 @@ class CDataGathererBuilder { return *this; } + std::shared_ptr buildSharedPtr() const { + return std::make_shared(m_GathererType, m_SummaryMode, m_Params, m_SummaryCountFieldName, + m_PartitionFieldValue, m_PersonFieldName, m_AttributeFieldName, + m_ValueFieldName, m_InfluenceFieldNames, m_SearchKey, m_Features, + m_StartTime, m_SampleCountOverride); + } + + CDataGathererBuilder& partitionFieldValue(const std::string& partitionFieldValue) { + m_PartitionFieldValue = partitionFieldValue; + return *this; + } + CDataGathererBuilder& personFieldName(std::string_view personFieldName) { m_PersonFieldName = personFieldName; return *this; From e82ff1a9a529cb78254f005ebb3dddf4fc382fb8 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 15:52:15 +0100 Subject: [PATCH 04/21] code compiles --- include/model/CBucketGatherer.h | 13 ++++ include/model/CDataGatherer.h | 44 ++++++------ include/model/CEventRateBucketGatherer.h | 16 ++--- include/model/CMetricBucketGatherer.h | 20 ++---- include/model/CModelFactory.h | 7 ++ lib/api/CAnomalyJob.cc | 7 +- lib/model/CCountingModelFactory.cc | 21 ++++-- lib/model/CDataGatherer.cc | 68 +++++++------------ lib/model/CEventRateBucketGatherer.cc | 45 +++++------- lib/model/CEventRateModelFactory.cc | 23 ++++--- lib/model/CEventRatePopulationModelFactory.cc | 29 +++++--- lib/model/CMetricBucketGatherer.cc | 66 ++++++++---------- lib/model/CMetricModelFactory.cc | 26 ++++--- lib/model/CMetricPopulationModelFactory.cc | 21 +++--- lib/model/CModelFactory.cc | 4 ++ .../CEventRatePopulationDataGathererTest.cc | 4 +- .../CMetricPopulationDataGathererTest.cc | 7 +- lib/model/unittest/ModelTestHelpers.h | 52 ++++++++------ 18 files changed, 244 insertions(+), 229 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 24c425673..cd90aa98f 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -132,6 +132,19 @@ class MODEL_EXPORT CBucketGatherer { using TTimeVec = std::vector; using TTimeVecCItr = TTimeVec::const_iterator; + struct SBucketGathererInitData { + static SBucketGathererInitData emptyData() { + return {.s_SummaryCountFieldName="", .s_PersonFieldName="", .s_AttributeFieldName="", .s_ValueFieldName="", .s_InfluenceFieldNames={}, .s_StartTime=0, .s_SampleOverrideCount=0}; + } + const std::string& s_SummaryCountFieldName; + const std::string& s_PersonFieldName; + const std::string& s_AttributeFieldName; + const std::string& s_ValueFieldName; + const TStrVec& s_InfluenceFieldNames; + core_t::TTime s_StartTime; + unsigned int s_SampleOverrideCount; + }; + public: static const std::string EVENTRATE_BUCKET_GATHERER_TAG; static const std::string METRIC_BUCKET_GATHERER_TAG; diff --git a/include/model/CDataGatherer.h b/include/model/CDataGatherer.h index b15e7514e..5dcb46c61 100644 --- a/include/model/CDataGatherer.h +++ b/include/model/CDataGatherer.h @@ -169,28 +169,30 @@ class MODEL_EXPORT CDataGatherer { CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& summaryCountFieldName, + // const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + // const std::string& personFieldName, + // const std::string& attributeFieldName, + // const std::string& valueFieldName, + // const TStrVec& influenceFieldNames, const CSearchKey& key, const TFeatureVec& features, - core_t::TTime startTime, - int sampleCountOverride); + // core_t::TTime startTime, + // int sampleCountOverride + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData); //! Construct from a state document. CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& summaryCountFieldName, + // const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + // const std::string& personFieldName, + // const std::string& attributeFieldName, + // const std::string& valueFieldName, + // const TStrVec& influenceFieldNames, const CSearchKey& key, + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); //! Create a copy that will result in the same persisted state as the @@ -675,19 +677,21 @@ class MODEL_EXPORT CDataGatherer { private: //! Restore state from supplied traverser. - bool acceptRestoreTraverser(const std::string& summaryCountFieldName, + bool acceptRestoreTraverser(/*const std::string& summaryCountFieldName, const std::string& personFieldName, const std::string& attributeFieldName, const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const TStrVec& influenceFieldNames,*/ + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); //! Restore a bucket gatherer from the supplied traverser. - bool restoreBucketGatherer(const std::string& summaryCountFieldName, + bool restoreBucketGatherer(/*const std::string& summaryCountFieldName, const std::string& personFieldName, const std::string& attributeFieldName, const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const TStrVec& influenceFieldNames,*/ + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); //! Persist a bucket gatherer by passing information to the supplied @@ -696,13 +700,7 @@ class MODEL_EXPORT CDataGatherer { //! Create the bucket specific data gatherer. void createBucketGatherer(model_t::EAnalysisCategory gathererType, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, - core_t::TTime startTime, - unsigned int sampleCountOverride); + const CBucketGatherer::SBucketGathererInitData& initData); private: //! The type of the bucket gatherer(s) used. diff --git a/include/model/CEventRateBucketGatherer.h b/include/model/CEventRateBucketGatherer.h index 3a1b5fafe..ca176552e 100644 --- a/include/model/CEventRateBucketGatherer.h +++ b/include/model/CEventRateBucketGatherer.h @@ -124,20 +124,22 @@ class MODEL_EXPORT CEventRateBucketGatherer final : public CBucketGatherer { //! \param[in] startTime The start of the time interval for which //! to gather data. CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData + /*const std::string& summaryCountFieldName, const std::string& personFieldName, const std::string& attributeFieldName, const std::string& valueFieldName, const TStrVec& influenceFieldNames, - core_t::TTime startTime); + core_t::TTime startTime*/); //! Construct from a state document. CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, + /*const std::string& summaryCountFieldName, const std::string& personFieldName, const std::string& attributeFieldName, const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const TStrVec& influenceFieldNames,*/ core::CStateRestoreTraverser& traverser); //! Create a copy that will result in the same persisted state as the @@ -444,11 +446,7 @@ class MODEL_EXPORT CEventRateBucketGatherer final : public CBucketGatherer { void startNewBucket(core_t::TTime time, bool skipUpdates) override; //! Initialize the field names collection. - void initializeFieldNames(const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const std::string& summaryCountFieldName, - const TStrVec& influenceFieldNames); + void initializeFieldNames(const CBucketGatherer::SBucketGathererInitData& initData); //! Initialize the feature data gatherers. void initializeFeatureData(); diff --git a/include/model/CMetricBucketGatherer.h b/include/model/CMetricBucketGatherer.h index d55e6292a..a577f5129 100644 --- a/include/model/CMetricBucketGatherer.h +++ b/include/model/CMetricBucketGatherer.h @@ -66,20 +66,11 @@ class MODEL_EXPORT CMetricBucketGatherer final : public CBucketGatherer { //! \param[in] startTime The start of the time interval for which //! to gather data. CMetricBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, - core_t::TTime startTime); + const CBucketGatherer::SBucketGathererInitData& initData); //! Construct from a state document. CMetricBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const CBucketGatherer::SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser); //! Create a copy that will result in the same persisted state as the @@ -266,9 +257,7 @@ class MODEL_EXPORT CMetricBucketGatherer final : public CBucketGatherer { //! 1) initializeFieldNamesPart1() //! 2) restore state //! 3) initializeFieldNamesPart2() - void initializeFieldNamesPart1(const std::string& personFieldName, - const std::string& attributeFieldName, - const TStrVec& influenceFieldNames); + void initializeFieldNamesPart1(const SBucketGathererInitData& initData); //! Initialize the field names collection. //! initializeFieldNamesPart1() must be called before this. @@ -277,8 +266,7 @@ class MODEL_EXPORT CMetricBucketGatherer final : public CBucketGatherer { //! 1) initializeFieldNamesPart1() //! 2) restore state //! 3) initializeFieldNamesPart2() - void initializeFieldNamesPart2(const std::string& valueFieldName, - const std::string& summaryCountFieldName); + void initializeFieldNamesPart2(const SBucketGathererInitData& initData); //! Initialize the feature data gatherers. void initializeFeatureData(); diff --git a/include/model/CModelFactory.h b/include/model/CModelFactory.h index 623f88db8..0b09ee72c 100644 --- a/include/model/CModelFactory.h +++ b/include/model/CModelFactory.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -104,6 +105,8 @@ class MODEL_EXPORT CModelFactory { using TStrDetectionRulePr = std::pair; using TStrDetectionRulePrVec = std::vector; using TStrDetectionRulePrVecCRef = std::reference_wrapper; + using TResourceMonitorCRef = std::reference_wrapper; + using TOptionalResourceMonitorCRef = std::optional; public: //! Wrapper around the model initialization data. @@ -373,6 +376,8 @@ class MODEL_EXPORT CModelFactory { //! Get the minimum seasonal variance scale, specific to the model virtual double minimumSeasonalVarianceScale() const = 0; + void resourceMonitor(const TResourceMonitorCRef& resourceMonitor) const; + protected: using TMultivariatePriorUPtrVec = std::vector; using TOptionalSearchKey = std::optional; @@ -455,6 +460,8 @@ class MODEL_EXPORT CModelFactory { //! A cache of influence calculators for collections of features. mutable TStrFeatureVecPrInfluenceCalculatorCPtrMap m_InfluenceCalculatorCache; + + mutable TOptionalResourceMonitorCRef m_ResourceMonitor; }; } } diff --git a/lib/api/CAnomalyJob.cc b/lib/api/CAnomalyJob.cc index 3a8c06be2..1a16a1ec9 100644 --- a/lib/api/CAnomalyJob.cc +++ b/lib/api/CAnomalyJob.cc @@ -1610,8 +1610,11 @@ CAnomalyJob::detectorForKey(bool isRestoring, << partition << '\'' << ", time " << time); LOG_TRACE(<< "Detector count " << m_Detectors.size()); - detector = ml::api::CAnomalyJob::makeDetector( - m_ModelConfig, m_Limits, partition, time, m_ModelConfig.factory(key)); + auto factory = m_ModelConfig.factory(key); + factory->resourceMonitor(resourceMonitor); + + detector = ml::api::CAnomalyJob::makeDetector(m_ModelConfig, m_Limits, + partition, time, factory); if (detector == nullptr) { // This should never happen as CAnomalyDetectorUtils::makeDetector() // contracts to never return NULL diff --git a/lib/model/CCountingModelFactory.cc b/lib/model/CCountingModelFactory.cc index ac11f9dcb..88760ac7a 100644 --- a/lib/model/CCountingModelFactory.cc +++ b/lib/model/CCountingModelFactory.cc @@ -61,19 +61,26 @@ CCountingModelFactory::makeModel(const SModelInitializationData& initData, CDataGatherer* CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { - return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), - m_SummaryCountFieldName, initData.s_PartitionFieldValue, - m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, - this->searchKey(), m_Features, initData.s_StartTime, 0); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{m_SummaryCountFieldName, + m_PersonFieldName, + EMPTY_STRING, + EMPTY_STRING, + {}, + initData.s_StartTime, + 0}; + return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, + this->modelParams(), initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CDataGatherer* CCountingModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - partitionFieldValue, m_PersonFieldName, EMPTY_STRING, - EMPTY_STRING, {}, this->searchKey(), traverser); + this->modelParams(), partitionFieldValue, + this->searchKey(), bucketGathererInitData, traverser); } CCountingModelFactory::TPriorPtr diff --git a/lib/model/CDataGatherer.cc b/lib/model/CDataGatherer.cc index d224feb9a..eb4a41c66 100644 --- a/lib/model/CDataGatherer.cc +++ b/lib/model/CDataGatherer.cc @@ -145,16 +145,10 @@ const std::size_t CDataGatherer::ESTIMATED_MEM_USAGE_PER_OVER_FIELD(1000); CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, const CSearchKey& key, const TFeatureVec& features, - core_t::TTime startTime, - int sampleCountOverride) + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData) : m_GathererType(gathererType), m_Features(detail::sanitize(features, gathererType)), m_SummaryMode(summaryMode), m_Params(modelParams), m_SearchKey(key), @@ -170,21 +164,20 @@ CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, m_Population(detail::isPopulation(gathererType)), m_UseNull(key.useNull()) { std::sort(m_Features.begin(), m_Features.end()); - this->createBucketGatherer(gathererType, summaryCountFieldName, - personFieldName, attributeFieldName, valueFieldName, - influenceFieldNames, startTime, sampleCountOverride); + this->createBucketGatherer(gathererType, bucketGathererInitData); } CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& summaryCountFieldName, + // const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + // const std::string& personFieldName, + // const std::string& attributeFieldName, + // const std::string& valueFieldName, + // const TStrVec& influenceFieldNames, const CSearchKey& key, + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) : m_GathererType(gathererType), m_SummaryMode(summaryMode), m_Params(modelParams), m_SearchKey(key), m_PartitionFieldValue(partitionFieldValue), @@ -197,10 +190,9 @@ CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, counter_t::E_TSADNumberNewAttributesNotAllowed, counter_t::E_TSADNumberNewAttributesRecycled), m_Population(detail::isPopulation(gathererType)), m_UseNull(key.useNull()) { - if (traverser.traverseSubLevel(std::bind( - &CDataGatherer::acceptRestoreTraverser, this, std::cref(summaryCountFieldName), - std::cref(personFieldName), std::cref(attributeFieldName), std::cref(valueFieldName), - std::cref(influenceFieldNames), std::placeholders::_1)) == false) { + if (traverser.traverseSubLevel(std::bind(&CDataGatherer::acceptRestoreTraverser, + this, std::cref(bucketGathererInitData), + std::placeholders::_1)) == false) { LOG_ERROR(<< "Failed to correctly restore data gatherer"); } } @@ -739,11 +731,12 @@ bool CDataGatherer::checkInvariants() const { return true; } -bool CDataGatherer::acceptRestoreTraverser(const std::string& summaryCountFieldName, +bool CDataGatherer::acceptRestoreTraverser(/*const std::string& summaryCountFieldName, const std::string& personFieldName, const std::string& attributeFieldName, const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const TStrVec& influenceFieldNames,*/ + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) { this->clear(); m_Features.clear(); @@ -775,34 +768,31 @@ bool CDataGatherer::acceptRestoreTraverser(const std::string& summaryCountFieldN RESTORE(BUCKET_GATHERER_TAG, traverser.traverseSubLevel(std::bind( &CDataGatherer::restoreBucketGatherer, this, - std::cref(summaryCountFieldName), std::cref(personFieldName), - std::cref(attributeFieldName), std::cref(valueFieldName), - std::cref(influenceFieldNames), std::placeholders::_1))) + std::cref(bucketGathererInitData), std::placeholders::_1))) } while (traverser.next()); return true; } -bool CDataGatherer::restoreBucketGatherer(const std::string& summaryCountFieldName, +bool CDataGatherer::restoreBucketGatherer(/*const std::string& summaryCountFieldName, const std::string& personFieldName, const std::string& attributeFieldName, const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const TStrVec& influenceFieldNames,*/ + const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) { do { const std::string& name = traverser.name(); if (name == CBucketGatherer::EVENTRATE_BUCKET_GATHERER_TAG) { m_BucketGatherer = std::make_unique( - *this, summaryCountFieldName, personFieldName, attributeFieldName, - valueFieldName, influenceFieldNames, traverser); + *this, bucketGathererInitData, traverser); if (m_BucketGatherer == nullptr) { LOG_ERROR(<< "Failed to create event rate bucket gatherer"); return false; } } else if (name == CBucketGatherer::METRIC_BUCKET_GATHERER_TAG) { m_BucketGatherer = std::make_unique( - *this, summaryCountFieldName, personFieldName, attributeFieldName, - valueFieldName, influenceFieldNames, traverser); + *this, bucketGathererInitData, traverser); if (m_BucketGatherer == nullptr) { LOG_ERROR(<< "Failed to create metric bucket gatherer"); return false; @@ -825,26 +815,16 @@ void CDataGatherer::persistBucketGatherers(core::CStatePersistInserter& inserter } void CDataGatherer::createBucketGatherer(model_t::EAnalysisCategory gathererType, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, - core_t::TTime startTime, - unsigned int sampleCountOverride) { + const CBucketGatherer::SBucketGathererInitData& initData) { switch (gathererType) { case model_t::E_EventRate: case model_t::E_PopulationEventRate: - m_BucketGatherer = std::make_unique( - *this, summaryCountFieldName, personFieldName, attributeFieldName, - valueFieldName, influenceFieldNames, startTime); + m_BucketGatherer = std::make_unique(*this, initData); break; case model_t::E_Metric: case model_t::E_PopulationMetric: - m_SampleCounts = std::make_unique(sampleCountOverride); - m_BucketGatherer = std::make_unique( - *this, summaryCountFieldName, personFieldName, attributeFieldName, - valueFieldName, influenceFieldNames, startTime); + m_SampleCounts = std::make_unique(initData.s_SampleOverrideCount); + m_BucketGatherer = std::make_unique(*this, initData); break; } } diff --git a/lib/model/CEventRateBucketGatherer.cc b/lib/model/CEventRateBucketGatherer.cc index c30dd1585..a01aa9479 100644 --- a/lib/model/CEventRateBucketGatherer.cc +++ b/lib/model/CEventRateBucketGatherer.cc @@ -727,30 +727,21 @@ void registerMemoryCallbacks() { } // unnamed:: CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, - core_t::TTime startTime) - : CBucketGatherer(dataGatherer, startTime, influenceFieldNames.size()), + const CBucketGatherer::SBucketGathererInitData& initData) + : CBucketGatherer(dataGatherer, + initData.s_StartTime, + initData.s_InfluenceFieldNames.size()), m_BeginInfluencingFields(0), m_BeginValueField(0), m_BeginSummaryFields(0) { - this->initializeFieldNames(personFieldName, attributeFieldName, valueFieldName, - summaryCountFieldName, influenceFieldNames); + this->initializeFieldNames(initData); this->initializeFeatureData(); } CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const CBucketGatherer::SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) - : CBucketGatherer(dataGatherer, 0, influenceFieldNames.size()), + : CBucketGatherer(dataGatherer, 0, initData.s_InfluenceFieldNames.size()), m_BeginInfluencingFields(0), m_BeginValueField(0), m_BeginSummaryFields(0) { - this->initializeFieldNames(personFieldName, attributeFieldName, valueFieldName, - summaryCountFieldName, influenceFieldNames); + this->initializeFieldNames(initData); if (traverser.traverseSubLevel(std::bind(&CEventRateBucketGatherer::acceptRestoreTraverser, this, std::placeholders::_1)) == false) { traverser.setBadState(); @@ -1501,23 +1492,19 @@ void CEventRateBucketGatherer::startNewBucket(core_t::TTime time, bool /*skipUpd }); } -void CEventRateBucketGatherer::initializeFieldNames(const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const std::string& summaryCountFieldName, - const TStrVec& influenceFieldNames) { - m_FieldNames.push_back(personFieldName); +void CEventRateBucketGatherer::initializeFieldNames(const CBucketGatherer::SBucketGathererInitData& initData) { + m_FieldNames.push_back(initData.s_PersonFieldName); if (m_DataGatherer.isPopulation()) { - m_FieldNames.push_back(attributeFieldName); + m_FieldNames.push_back(initData.s_AttributeFieldName); } m_BeginInfluencingFields = m_FieldNames.size(); - m_FieldNames.insert(m_FieldNames.end(), influenceFieldNames.begin(), - influenceFieldNames.end()); + m_FieldNames.insert(m_FieldNames.end(), initData.s_InfluenceFieldNames.begin(), + initData.s_InfluenceFieldNames.end()); m_BeginValueField = m_FieldNames.size(); - if (!valueFieldName.empty()) { - m_FieldNames.push_back(valueFieldName); + if (!initData.s_ValueFieldName.empty()) { + m_FieldNames.push_back(initData.s_ValueFieldName); } m_BeginSummaryFields = m_FieldNames.size(); @@ -1525,7 +1512,7 @@ void CEventRateBucketGatherer::initializeFieldNames(const std::string& personFie case model_t::E_None: break; case model_t::E_Manual: - m_FieldNames.push_back(summaryCountFieldName); + m_FieldNames.push_back(initData.s_SummaryCountFieldName); break; } diff --git a/lib/model/CEventRateModelFactory.cc b/lib/model/CEventRateModelFactory.cc index 2ddb927d4..a3e8423b6 100644 --- a/lib/model/CEventRateModelFactory.cc +++ b/lib/model/CEventRateModelFactory.cc @@ -92,22 +92,27 @@ CEventRateModelFactory::makeModel(const SModelInitializationData& initData, CDataGatherer* CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, + m_PersonFieldName, + EMPTY_STRING, + m_ValueFieldName, + m_InfluenceFieldNames, + initData.s_StartTime, + initData.s_SampleOverrideCount}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - initData.s_PartitionFieldValue, m_PersonFieldName, - EMPTY_STRING /*AttributeFieldName*/, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), m_Features, - initData.s_StartTime, initData.s_SampleOverrideCount); + this->modelParams(), initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CDataGatherer* CEventRateModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - partitionFieldValue, m_PersonFieldName, - EMPTY_STRING /*AttributeFieldName*/, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), traverser); + this->modelParams(), partitionFieldValue, + this->searchKey(), bucketGathererInitData, traverser); } CEventRateModelFactory::TPriorPtr diff --git a/lib/model/CEventRatePopulationModelFactory.cc b/lib/model/CEventRatePopulationModelFactory.cc index 39042ac74..b5cbfea26 100644 --- a/lib/model/CEventRatePopulationModelFactory.cc +++ b/lib/model/CEventRatePopulationModelFactory.cc @@ -93,21 +93,34 @@ CEventRatePopulationModelFactory::makeModel(const SModelInitializationData& init CDataGatherer* CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { + CBucketGatherer::SBucketGathererInitData const bucketGathererInitData{ + .s_SummaryCountFieldName=m_SummaryCountFieldName, + .s_PersonFieldName=m_PersonFieldName, + .s_AttributeFieldName=m_AttributeFieldName, + .s_ValueFieldName=m_ValueFieldName, + .s_InfluenceFieldNames=m_InfluenceFieldNames, + .s_StartTime=initData.s_StartTime, + .s_SampleOverrideCount=0}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - initData.s_PartitionFieldValue, m_PersonFieldName, - m_AttributeFieldName, m_ValueFieldName, m_InfluenceFieldNames, - this->searchKey(), m_Features, initData.s_StartTime, 0); + this->modelParams(), + initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CDataGatherer* CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { + CBucketGatherer::SBucketGathererInitData const bucketGathererInitData{ + .s_SummaryCountFieldName=m_SummaryCountFieldName, + .s_PersonFieldName=m_PersonFieldName, + .s_AttributeFieldName=m_AttributeFieldName, + .s_ValueFieldName=m_ValueFieldName, + .s_InfluenceFieldNames=m_InfluenceFieldNames, + .s_StartTime=0, + .s_SampleOverrideCount=0}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - partitionFieldValue, m_PersonFieldName, - m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), traverser); + this->modelParams(), partitionFieldValue, + this->searchKey(), bucketGathererInitData, traverser); } CEventRatePopulationModelFactory::TPriorPtr diff --git a/lib/model/CMetricBucketGatherer.cc b/lib/model/CMetricBucketGatherer.cc index 80951cb98..06439cfef 100644 --- a/lib/model/CMetricBucketGatherer.cc +++ b/lib/model/CMetricBucketGatherer.cc @@ -925,35 +925,28 @@ struct SReleaseMemory { } // unnamed:: CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, - core_t::TTime startTime) - : CBucketGatherer(dataGatherer, startTime, influenceFieldNames.size()), - m_ValueFieldName(valueFieldName), m_BeginInfluencingFields(0), + const CBucketGatherer::SBucketGathererInitData& initData) + : CBucketGatherer(dataGatherer, + initData.s_StartTime, + initData.s_InfluenceFieldNames.size()), + m_ValueFieldName(initData.s_ValueFieldName), m_BeginInfluencingFields(0), m_BeginValueFields(0) { - this->initializeFieldNamesPart1(personFieldName, attributeFieldName, influenceFieldNames); - this->initializeFieldNamesPart2(valueFieldName, summaryCountFieldName); + this->initializeFieldNamesPart1(initData); + this->initializeFieldNamesPart2(initData); this->initializeFeatureData(); } CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, - const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, + const CBucketGatherer::SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) - : CBucketGatherer(dataGatherer, 0, influenceFieldNames.size()), - m_ValueFieldName(valueFieldName), m_BeginValueFields(0) { - this->initializeFieldNamesPart1(personFieldName, attributeFieldName, influenceFieldNames); + : CBucketGatherer(dataGatherer, 0, initData.s_InfluenceFieldNames.size()), + m_ValueFieldName(initData.s_ValueFieldName), m_BeginValueFields(0) { + this->initializeFieldNamesPart1(initData); if (traverser.traverseSubLevel(std::bind(&CMetricBucketGatherer::acceptRestoreTraverser, this, std::placeholders::_1)) == false) { traverser.setBadState(); } else { - this->initializeFieldNamesPart2(valueFieldName, summaryCountFieldName); + this->initializeFieldNamesPart2(initData); } } @@ -1505,44 +1498,41 @@ void CMetricBucketGatherer::startNewBucket(core_t::TTime time, bool skipUpdates) }); } -void CMetricBucketGatherer::initializeFieldNamesPart1(const std::string& personFieldName, - const std::string& attributeFieldName, - const TStrVec& influenceFieldNames) { +void CMetricBucketGatherer::initializeFieldNamesPart1(const SBucketGathererInitData& initData) { switch (m_DataGatherer.summaryMode()) { case model_t::E_None: m_FieldNames.reserve(2 + static_cast(m_DataGatherer.isPopulation()) + - influenceFieldNames.size()); - m_FieldNames.push_back(personFieldName); + initData.s_InfluenceFieldNames.size()); + m_FieldNames.push_back(initData.s_PersonFieldName); if (m_DataGatherer.isPopulation()) - m_FieldNames.push_back(attributeFieldName); + m_FieldNames.push_back(initData.s_AttributeFieldName); m_BeginInfluencingFields = m_FieldNames.size(); - m_FieldNames.insert(m_FieldNames.end(), influenceFieldNames.begin(), - influenceFieldNames.end()); + m_FieldNames.insert(m_FieldNames.end(), initData.s_InfluenceFieldNames.begin(), + initData.s_InfluenceFieldNames.end()); m_BeginValueFields = m_FieldNames.size(); break; case model_t::E_Manual: m_FieldNames.reserve(3 + static_cast(m_DataGatherer.isPopulation()) + - influenceFieldNames.size()); - m_FieldNames.push_back(personFieldName); + initData.s_InfluenceFieldNames.size()); + m_FieldNames.push_back(initData.s_PersonFieldName); if (m_DataGatherer.isPopulation()) - m_FieldNames.push_back(attributeFieldName); + m_FieldNames.push_back(initData.s_AttributeFieldName); m_BeginInfluencingFields = m_FieldNames.size(); - m_FieldNames.insert(m_FieldNames.end(), influenceFieldNames.begin(), - influenceFieldNames.end()); + m_FieldNames.insert(m_FieldNames.end(), initData.s_InfluenceFieldNames.begin(), + initData.s_InfluenceFieldNames.end()); m_BeginValueFields = m_FieldNames.size(); break; - }; + } } -void CMetricBucketGatherer::initializeFieldNamesPart2(const std::string& valueFieldName, - const std::string& summaryCountFieldName) { +void CMetricBucketGatherer::initializeFieldNamesPart2(const SBucketGathererInitData& initData) { switch (m_DataGatherer.summaryMode()) { case model_t::E_None: - m_FieldNames.push_back(valueFieldName); + m_FieldNames.push_back(initData.s_ValueFieldName); break; case model_t::E_Manual: - m_FieldNames.push_back(summaryCountFieldName); - m_FieldNames.push_back(valueFieldName); + m_FieldNames.push_back(initData.s_SummaryCountFieldName); + m_FieldNames.push_back(initData.s_ValueFieldName); m_DataGatherer.determineMetricCategory(m_FieldMetricCategories); break; }; diff --git a/lib/model/CMetricModelFactory.cc b/lib/model/CMetricModelFactory.cc index e627bfc02..7c6b5fcc2 100644 --- a/lib/model/CMetricModelFactory.cc +++ b/lib/model/CMetricModelFactory.cc @@ -92,21 +92,27 @@ CMetricModelFactory::makeModel(const SModelInitializationData& initData, CDataGatherer* CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { - return new CDataGatherer(model_t::E_Metric, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - initData.s_PartitionFieldValue, m_PersonFieldName, - EMPTY_STRING /*AttributeFieldName*/, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), m_Features, - initData.s_StartTime, initData.s_SampleOverrideCount); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, + m_PersonFieldName, + EMPTY_STRING /*AttributeFieldName*/, + m_ValueFieldName, + m_InfluenceFieldNames, + initData.s_StartTime, + initData.s_SampleOverrideCount}; + return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), + initData.s_PartitionFieldValue, this->searchKey(), + m_Features, bucketGathererInitData); } CDataGatherer* CMetricModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { - return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), - m_SummaryCountFieldName, partitionFieldValue, m_PersonFieldName, - EMPTY_STRING /*AttributeFieldName*/, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), traverser); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; + return new CDataGatherer(model_t::E_Metric, m_SummaryMode, + this->modelParams(), partitionFieldValue, + this->searchKey(), bucketGathererInitData, traverser); } CMetricModelFactory::TPriorPtr diff --git a/lib/model/CMetricPopulationModelFactory.cc b/lib/model/CMetricPopulationModelFactory.cc index a758937dd..a32989f5d 100644 --- a/lib/model/CMetricPopulationModelFactory.cc +++ b/lib/model/CMetricPopulationModelFactory.cc @@ -92,22 +92,25 @@ CMetricPopulationModelFactory::makeModel(const SModelInitializationData& initDat CDataGatherer* CMetricPopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, m_PersonFieldName, + m_AttributeFieldName, m_ValueFieldName, + m_InfluenceFieldNames, initData.s_StartTime, + initData.s_SampleOverrideCount}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - initData.s_PartitionFieldValue, m_PersonFieldName, - m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), m_Features, - initData.s_StartTime, initData.s_SampleOverrideCount); + this->modelParams(), initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CDataGatherer* CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, m_PersonFieldName, m_AttributeFieldName, m_ValueFieldName, + m_InfluenceFieldNames, 0, 0}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, - this->modelParams(), m_SummaryCountFieldName, - partitionFieldValue, m_PersonFieldName, - m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, this->searchKey(), traverser); + this->modelParams(), + partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } CMetricPopulationModelFactory::TPriorPtr diff --git a/lib/model/CModelFactory.cc b/lib/model/CModelFactory.cc index 70737d556..d051e88ba 100644 --- a/lib/model/CModelFactory.cc +++ b/lib/model/CModelFactory.cc @@ -291,6 +291,10 @@ void CModelFactory::annotationsEnabled(bool enabled) { m_ModelParams.s_AnnotationsEnabled = enabled; } +void CModelFactory::resourceMonitor(const TResourceMonitorCRef& resourceMonitor) const { + m_ResourceMonitor = resourceMonitor; +} + CModelFactory::TInterimBucketCorrectorPtr CModelFactory::interimBucketCorrector() const { TInterimBucketCorrectorPtr result{m_InterimBucketCorrector.lock()}; if (result == nullptr) { diff --git a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc index e8e25c752..91c7f8775 100644 --- a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc +++ b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc @@ -783,10 +783,10 @@ void testPersistDataGatherer(const CDataGatherer& origDataGatherer, std::istringstream origJsonStrm("{\"topLevel\" : " + origJson.str() + "}"); core::CJsonStateRestoreTraverser traverser(origJsonStrm); + auto bucketGathererInitData = CBucketGatherer::SBucketGathererInitData::emptyData(); CDataGatherer restoredDataGatherer(model_t::E_PopulationEventRate, model_t::E_None, params, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, {}, searchKey, traverser); + searchKey, bucketGathererInitData, traverser); // The Json representation of the new data gatherer should be the same as the // original diff --git a/lib/model/unittest/CMetricPopulationDataGathererTest.cc b/lib/model/unittest/CMetricPopulationDataGathererTest.cc index 6c4be3280..b5e6279e2 100644 --- a/lib/model/unittest/CMetricPopulationDataGathererTest.cc +++ b/lib/model/unittest/CMetricPopulationDataGathererTest.cc @@ -1028,11 +1028,10 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { // The traverser expects the state json in a embedded document std::stringstream origJsonStrm{"{\"topLevel\" : " + origJson.str() + "}"}; core::CJsonStateRestoreTraverser traverser(origJsonStrm); - + auto bucketInitData = CBucketGatherer::SBucketGathererInitData::emptyData(); CDataGatherer restoredDataGatherer(model_t::E_PopulationMetric, - model_t::E_None, params, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, {}, searchKey, traverser); + model_t::E_None, params, + EMPTY_STRING, searchKey, bucketInitData, traverser); // The JSON representation of the new data gatherer should be the same as the // original diff --git a/lib/model/unittest/ModelTestHelpers.h b/lib/model/unittest/ModelTestHelpers.h index d75ca990d..5fbd4cdce 100644 --- a/lib/model/unittest/ModelTestHelpers.h +++ b/lib/model/unittest/ModelTestHelpers.h @@ -45,9 +45,17 @@ static void testPersistence(const SModelParams& params, std::istringstream origJsonStrm{"{\"topLevel\" : " + origJson.str() + "}"}; core::CJsonStateRestoreTraverser traverser(origJsonStrm); - CDataGatherer restoredGatherer(category, model_t::E_None, params, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, {}, KEY, traverser); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + EMPTY_STRING, + EMPTY_STRING, + EMPTY_STRING, + EMPTY_STRING, + {}, + 0, + 0}; + + CDataGatherer restoredGatherer(category, model_t::E_None, params, + EMPTY_STRING, KEY, bucketGathererInitData, traverser); BOOST_REQUIRE_EQUAL(origGatherer.checksum(), restoredGatherer.checksum()); @@ -96,19 +104,18 @@ class CDataGathererBuilder { m_SearchKey(searchKey), m_GathererType(gathererType) {} CDataGatherer build() const { - return {m_GathererType, - m_SummaryMode, - m_Params, - m_SummaryCountFieldName, - m_PartitionFieldValue, - m_PersonFieldName, - m_AttributeFieldName, - m_ValueFieldName, - m_InfluenceFieldNames, - m_SearchKey, + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, + m_PersonFieldName, + m_AttributeFieldName, + m_ValueFieldName, + m_InfluenceFieldNames, + m_StartTime, + static_cast(m_SampleCountOverride)}; + return {m_GathererType, m_SummaryMode, m_Params, + m_PartitionFieldValue, m_SearchKey, m_Features, - m_StartTime, - m_SampleCountOverride}; + bucketGathererInitData}; } std::shared_ptr buildSharedPtr() const { @@ -125,10 +132,17 @@ class CDataGathererBuilder { } std::shared_ptr buildSharedPtr() const { - return std::make_shared(m_GathererType, m_SummaryMode, m_Params, m_SummaryCountFieldName, - m_PartitionFieldValue, m_PersonFieldName, m_AttributeFieldName, - m_ValueFieldName, m_InfluenceFieldNames, m_SearchKey, m_Features, - m_StartTime, m_SampleCountOverride); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, + m_PersonFieldName, + m_AttributeFieldName, + m_ValueFieldName, + m_InfluenceFieldNames, + m_StartTime, + static_cast(m_SampleCountOverride)}; + return std::make_shared(m_GathererType, m_SummaryMode, m_Params, + m_PartitionFieldValue, m_SearchKey, m_Features, + bucketGathererInitData); } CDataGathererBuilder& partitionFieldValue(const std::string& partitionFieldValue) { From fab3ad9b82353b3bc6861f84a3c34b26fc3ca1bf Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 16:03:49 +0100 Subject: [PATCH 05/21] resource monitor is added to bucket gatherer init data --- include/model/CBucketGatherer.h | 2 ++ include/model/CModelFactory.h | 2 ++ lib/model/CCountingModelFactory.cc | 7 ++++--- lib/model/CEventRateModelFactory.cc | 5 +++-- lib/model/CEventRatePopulationModelFactory.cc | 5 +++-- lib/model/CMetricModelFactory.cc | 5 +++-- lib/model/CMetricPopulationModelFactory.cc | 4 ++-- lib/model/CModelFactory.cc | 3 +++ 8 files changed, 22 insertions(+), 11 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index cd90aa98f..0a00dd9c4 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -131,6 +131,7 @@ class MODEL_EXPORT CBucketGatherer { using TMetricCategoryVec = std::vector; using TTimeVec = std::vector; using TTimeVecCItr = TTimeVec::const_iterator; + using TOptionalResourceMonitorCRef = std::optional>; struct SBucketGathererInitData { static SBucketGathererInitData emptyData() { @@ -143,6 +144,7 @@ class MODEL_EXPORT CBucketGatherer { const TStrVec& s_InfluenceFieldNames; core_t::TTime s_StartTime; unsigned int s_SampleOverrideCount; + TOptionalResourceMonitorCRef s_ResourceMonitor; }; public: diff --git a/include/model/CModelFactory.h b/include/model/CModelFactory.h index 0b09ee72c..264fd8a09 100644 --- a/include/model/CModelFactory.h +++ b/include/model/CModelFactory.h @@ -378,6 +378,8 @@ class MODEL_EXPORT CModelFactory { void resourceMonitor(const TResourceMonitorCRef& resourceMonitor) const; + TOptionalResourceMonitorCRef resourceMonitor() const; + protected: using TMultivariatePriorUPtrVec = std::vector; using TOptionalSearchKey = std::optional; diff --git a/lib/model/CCountingModelFactory.cc b/lib/model/CCountingModelFactory.cc index 88760ac7a..b375b8dad 100644 --- a/lib/model/CCountingModelFactory.cc +++ b/lib/model/CCountingModelFactory.cc @@ -61,13 +61,14 @@ CCountingModelFactory::makeModel(const SModelInitializationData& initData, CDataGatherer* CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { - CBucketGatherer::SBucketGathererInitData bucketGathererInitData{m_SummaryCountFieldName, + const CBucketGatherer::SBucketGathererInitData bucketGathererInitData{m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, initData.s_StartTime, - 0}; + 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -77,7 +78,7 @@ CDataGatherer* CCountingModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0, this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CEventRateModelFactory.cc b/lib/model/CEventRateModelFactory.cc index a3e8423b6..b67763f57 100644 --- a/lib/model/CEventRateModelFactory.cc +++ b/lib/model/CEventRateModelFactory.cc @@ -99,7 +99,8 @@ CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& init m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount}; + initData.s_SampleOverrideCount, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -109,7 +110,7 @@ CDataGatherer* CEventRateModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0, this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CEventRatePopulationModelFactory.cc b/lib/model/CEventRatePopulationModelFactory.cc index b5cbfea26..7bf0fa6d0 100644 --- a/lib/model/CEventRatePopulationModelFactory.cc +++ b/lib/model/CEventRatePopulationModelFactory.cc @@ -100,7 +100,8 @@ CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitialization .s_ValueFieldName=m_ValueFieldName, .s_InfluenceFieldNames=m_InfluenceFieldNames, .s_StartTime=initData.s_StartTime, - .s_SampleOverrideCount=0}; + .s_SampleOverrideCount=0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, @@ -117,7 +118,7 @@ CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionF .s_ValueFieldName=m_ValueFieldName, .s_InfluenceFieldNames=m_InfluenceFieldNames, .s_StartTime=0, - .s_SampleOverrideCount=0}; + .s_SampleOverrideCount=0, this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CMetricModelFactory.cc b/lib/model/CMetricModelFactory.cc index 7c6b5fcc2..5fd4ef682 100644 --- a/lib/model/CMetricModelFactory.cc +++ b/lib/model/CMetricModelFactory.cc @@ -99,7 +99,8 @@ CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initDat m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount}; + initData.s_SampleOverrideCount, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -109,7 +110,7 @@ CDataGatherer* CMetricModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0, this->resourceMonitor()}; return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CMetricPopulationModelFactory.cc b/lib/model/CMetricPopulationModelFactory.cc index a32989f5d..16a3c2725 100644 --- a/lib/model/CMetricPopulationModelFactory.cc +++ b/lib/model/CMetricPopulationModelFactory.cc @@ -96,7 +96,7 @@ CMetricPopulationModelFactory::makeDataGatherer(const SGathererInitializationDat m_SummaryCountFieldName, m_PersonFieldName, m_AttributeFieldName, m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount}; + initData.s_SampleOverrideCount,this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -107,7 +107,7 @@ CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFiel core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, 0, 0}; + m_InfluenceFieldNames, 0, 0, this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CModelFactory.cc b/lib/model/CModelFactory.cc index d051e88ba..81184ed22 100644 --- a/lib/model/CModelFactory.cc +++ b/lib/model/CModelFactory.cc @@ -294,6 +294,9 @@ void CModelFactory::annotationsEnabled(bool enabled) { void CModelFactory::resourceMonitor(const TResourceMonitorCRef& resourceMonitor) const { m_ResourceMonitor = resourceMonitor; } +CModelFactory::TOptionalResourceMonitorCRef CModelFactory::resourceMonitor() const { + return m_ResourceMonitor; +} CModelFactory::TInterimBucketCorrectorPtr CModelFactory::interimBucketCorrector() const { TInterimBucketCorrectorPtr result{m_InterimBucketCorrector.lock()}; From 56a8c777fdc393c4918125882f2e99b39ce9bb71 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 16:04:42 +0100 Subject: [PATCH 06/21] formatting --- include/model/CBucketGatherer.h | 11 +- lib/model/CCountingModelFactory.cc | 17 +-- lib/model/CEventRateModelFactory.cc | 11 +- lib/model/CEventRatePopulationModelFactory.cc | 34 +++--- lib/model/CMetricModelFactory.cc | 11 +- lib/model/CMetricPopulationModelFactory.cc | 22 ++-- .../unittest/CEventRateDataGathererTest.cc | 113 ++++++++++-------- .../CEventRatePopulationDataGathererTest.cc | 8 +- lib/model/unittest/CMetricDataGathererTest.cc | 8 +- .../CMetricPopulationDataGathererTest.cc | 5 +- lib/model/unittest/ModelTestHelpers.h | 21 ++-- 11 files changed, 153 insertions(+), 108 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 0a00dd9c4..1beaaebca 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -131,11 +131,18 @@ class MODEL_EXPORT CBucketGatherer { using TMetricCategoryVec = std::vector; using TTimeVec = std::vector; using TTimeVecCItr = TTimeVec::const_iterator; - using TOptionalResourceMonitorCRef = std::optional>; + using TOptionalResourceMonitorCRef = + std::optional>; struct SBucketGathererInitData { static SBucketGathererInitData emptyData() { - return {.s_SummaryCountFieldName="", .s_PersonFieldName="", .s_AttributeFieldName="", .s_ValueFieldName="", .s_InfluenceFieldNames={}, .s_StartTime=0, .s_SampleOverrideCount=0}; + return {.s_SummaryCountFieldName = "", + .s_PersonFieldName = "", + .s_AttributeFieldName = "", + .s_ValueFieldName = "", + .s_InfluenceFieldNames = {}, + .s_StartTime = 0, + .s_SampleOverrideCount = 0}; } const std::string& s_SummaryCountFieldName; const std::string& s_PersonFieldName; diff --git a/lib/model/CCountingModelFactory.cc b/lib/model/CCountingModelFactory.cc index b375b8dad..e4895a213 100644 --- a/lib/model/CCountingModelFactory.cc +++ b/lib/model/CCountingModelFactory.cc @@ -61,14 +61,15 @@ CCountingModelFactory::makeModel(const SModelInitializationData& initData, CDataGatherer* CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { - const CBucketGatherer::SBucketGathererInitData bucketGathererInitData{m_SummaryCountFieldName, - m_PersonFieldName, - EMPTY_STRING, - EMPTY_STRING, - {}, - initData.s_StartTime, - 0, - this->resourceMonitor()}; + const CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + m_SummaryCountFieldName, + m_PersonFieldName, + EMPTY_STRING, + EMPTY_STRING, + {}, + initData.s_StartTime, + 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); diff --git a/lib/model/CEventRateModelFactory.cc b/lib/model/CEventRateModelFactory.cc index b67763f57..3090408c4 100644 --- a/lib/model/CEventRateModelFactory.cc +++ b/lib/model/CEventRateModelFactory.cc @@ -100,7 +100,7 @@ CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& init m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount, - this->resourceMonitor()}; + this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -110,7 +110,14 @@ CDataGatherer* CEventRateModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0, this->resourceMonitor()}; + m_SummaryCountFieldName, + m_PersonFieldName, + EMPTY_STRING, + m_ValueFieldName, + m_InfluenceFieldNames, + 0, + 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CEventRatePopulationModelFactory.cc b/lib/model/CEventRatePopulationModelFactory.cc index 7bf0fa6d0..89de455c2 100644 --- a/lib/model/CEventRatePopulationModelFactory.cc +++ b/lib/model/CEventRatePopulationModelFactory.cc @@ -94,17 +94,16 @@ CEventRatePopulationModelFactory::makeModel(const SModelInitializationData& init CDataGatherer* CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData const bucketGathererInitData{ - .s_SummaryCountFieldName=m_SummaryCountFieldName, - .s_PersonFieldName=m_PersonFieldName, - .s_AttributeFieldName=m_AttributeFieldName, - .s_ValueFieldName=m_ValueFieldName, - .s_InfluenceFieldNames=m_InfluenceFieldNames, - .s_StartTime=initData.s_StartTime, - .s_SampleOverrideCount=0, - this->resourceMonitor()}; + .s_SummaryCountFieldName = m_SummaryCountFieldName, + .s_PersonFieldName = m_PersonFieldName, + .s_AttributeFieldName = m_AttributeFieldName, + .s_ValueFieldName = m_ValueFieldName, + .s_InfluenceFieldNames = m_InfluenceFieldNames, + .s_StartTime = initData.s_StartTime, + .s_SampleOverrideCount = 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, - this->modelParams(), - initData.s_PartitionFieldValue, + this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); } @@ -112,13 +111,14 @@ CDataGatherer* CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData const bucketGathererInitData{ - .s_SummaryCountFieldName=m_SummaryCountFieldName, - .s_PersonFieldName=m_PersonFieldName, - .s_AttributeFieldName=m_AttributeFieldName, - .s_ValueFieldName=m_ValueFieldName, - .s_InfluenceFieldNames=m_InfluenceFieldNames, - .s_StartTime=0, - .s_SampleOverrideCount=0, this->resourceMonitor()}; + .s_SummaryCountFieldName = m_SummaryCountFieldName, + .s_PersonFieldName = m_PersonFieldName, + .s_AttributeFieldName = m_AttributeFieldName, + .s_ValueFieldName = m_ValueFieldName, + .s_InfluenceFieldNames = m_InfluenceFieldNames, + .s_StartTime = 0, + .s_SampleOverrideCount = 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CMetricModelFactory.cc b/lib/model/CMetricModelFactory.cc index 5fd4ef682..3357925f0 100644 --- a/lib/model/CMetricModelFactory.cc +++ b/lib/model/CMetricModelFactory.cc @@ -100,7 +100,7 @@ CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initDat m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount, - this->resourceMonitor()}; + this->resourceMonitor()}; return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -110,7 +110,14 @@ CDataGatherer* CMetricModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0, this->resourceMonitor()}; + m_SummaryCountFieldName, + m_PersonFieldName, + EMPTY_STRING, + m_ValueFieldName, + m_InfluenceFieldNames, + 0, + 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CMetricPopulationModelFactory.cc b/lib/model/CMetricPopulationModelFactory.cc index 16a3c2725..cced96384 100644 --- a/lib/model/CMetricPopulationModelFactory.cc +++ b/lib/model/CMetricPopulationModelFactory.cc @@ -93,10 +93,10 @@ CMetricPopulationModelFactory::makeModel(const SModelInitializationData& initDat CDataGatherer* CMetricPopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, - m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount,this->resourceMonitor()}; + m_SummaryCountFieldName, m_PersonFieldName, + m_AttributeFieldName, m_ValueFieldName, + m_InfluenceFieldNames, initData.s_StartTime, + initData.s_SampleOverrideCount, this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -106,11 +106,17 @@ CDataGatherer* CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, 0, 0, this->resourceMonitor()}; + m_SummaryCountFieldName, + m_PersonFieldName, + m_AttributeFieldName, + m_ValueFieldName, + m_InfluenceFieldNames, + 0, + 0, + this->resourceMonitor()}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, - this->modelParams(), - partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); + this->modelParams(), partitionFieldValue, + this->searchKey(), bucketGathererInitData, traverser); } CMetricPopulationModelFactory::TPriorPtr diff --git a/lib/model/unittest/CEventRateDataGathererTest.cc b/lib/model/unittest/CEventRateDataGathererTest.cc index 62d7db275..96b15c590 100644 --- a/lib/model/unittest/CEventRateDataGathererTest.cc +++ b/lib/model/unittest/CEventRateDataGathererTest.cc @@ -63,8 +63,6 @@ namespace { const CSearchKey key; const std::string EMPTY_STRING; - - std::size_t addPerson(CDataGatherer& gatherer, CResourceMonitor& resourceMonitor, const std::string& p, @@ -138,7 +136,7 @@ void addArrival(CDataGatherer& gatherer, CDataGatherer::TStrCPtrVec fieldValues; fieldValues.push_back(&person); - for (const auto & influencer : influencers) { + for (const auto& influencer : influencers) { fieldValues.push_back(&influencer); } @@ -168,7 +166,8 @@ void testInfluencerPerFeature(const model_t::EFeature feature, features.push_back(feature); TStrVec influencerFieldNames; influencerFieldNames.emplace_back("IF1"); - CDataGatherer gatherer = CDataGathererBuilder(model_t::E_EventRate, features, params, key, startTime) + CDataGatherer gatherer = CDataGathererBuilder(model_t::E_EventRate, features, + params, key, startTime) .influenceFieldNames(influencerFieldNames) .valueFieldName(valueField) .build(); @@ -495,9 +494,9 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeries, CTestFixture) { 10000 // sentinel }; - std::array const expectedPersonCounts{std::string("[(0, 6)]"), std::string("[(0, 3)]"), - std::string("[(0, 2)]"), std::string("[(0, 0)]"), - std::string("[(0, 3)]")}; + std::array const expectedPersonCounts{ + std::string("[(0, 6)]"), std::string("[(0, 3)]"), std::string("[(0, 2)]"), + std::string("[(0, 0)]"), std::string("[(0, 3)]")}; std::array const expectedPersonNonZeroCounts{ std::string("[(0, 6)]"), std::string("[(0, 3)]"), @@ -639,7 +638,7 @@ BOOST_FIXTURE_TEST_CASE(testMultipleSeries, CTestFixture) { constexpr core_t::TTime startTime = 0; constexpr core_t::TTime bucketLength = 600; - const std::vector data1 = { + const std::vector data1 = { 1, 15, 180, 190, 400, 550, // bucket 1 600, 799, @@ -745,7 +744,7 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { BOOST_REQUIRE_EQUAL(4, addPerson(expectedGatherer, m_ResourceMonitor, "p7")); BOOST_REQUIRE_EQUAL(5, addPerson(expectedGatherer, m_ResourceMonitor, "p8")); - constexpr std::array expectedCounts = {5, 2, 0, + constexpr std::array expectedCounts = {5, 2, 0, 5, 7, 10}; for (std::size_t i = 0; i < std::size(expectedCounts); ++i) { for (core_t::TTime time = 0; time < expectedCounts[i]; ++time) { @@ -822,7 +821,7 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeriesOutOfOrderFinalResult, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - const std::array data = { + const std::array data = { 1, 180, 1200, 190, 400, 600, // bucket 1, 2 & 3 550, 799, 1199, @@ -970,7 +969,7 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeriesOutOfOrderInterimResult, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - constexpr std::array data = { + constexpr std::array data = { 1, 1200, 600, // bucket 1, 3 & 2 1199, @@ -1135,9 +1134,9 @@ BOOST_FIXTURE_TEST_CASE(testMultipleSeriesOutOfOrderFinalResult, CTestFixture) { .build(); testGathererMultipleSeries( - STestTimes{.s_StartTime= startTime, .s_BucketLength= bucketLength}, - STestData{.data1 =data1, .data2=data2}, expectedPersonCounts, - params, latencyTime, gatherer, m_ResourceMonitor); + STestTimes{.s_StartTime = startTime, .s_BucketLength = bucketLength}, + STestData{.data1 = data1, .data2 = data2}, expectedPersonCounts, + params, latencyTime, gatherer, m_ResourceMonitor); } { @@ -1158,7 +1157,7 @@ BOOST_FIXTURE_TEST_CASE(testArrivalBeforeLatencyWindowIsIgnored, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - constexpr std::array data = { + constexpr std::array data = { 1800, // Bucket 4, thus bucket 1's values are already out of latency window 1 // Bucket 1 }; @@ -1199,7 +1198,7 @@ BOOST_FIXTURE_TEST_CASE(testResetBucketGivenSingleSeries, CTestFixture) { SModelParams params(bucketLength); params.s_LatencyBuckets = latencyBuckets; - constexpr std::array data = { + constexpr std::array data = { 100, 300, // Bucket 1 600, 800, @@ -1331,7 +1330,7 @@ BOOST_FIXTURE_TEST_CASE(testResetBucketGivenBucketNotAvailable, CTestFixture) { } BOOST_FIXTURE_TEST_CASE(testInfluencerBucketStatistics, CTestFixture) { - constexpr std::array data = { + constexpr std::array data = { 1, 15, 180, 190, 400, 550, // bucket 1 600, 799, @@ -1408,7 +1407,7 @@ class CDistinctStringsTestFixture : public CTestFixture { // Helper that creates a SEventRateFeatureData object, populates it using the distinct count method, // and checks that its print() output matches the expected value. static void verifyDistinctCountFeature(const CUniqueStringFeatureData& data, - const std::string& expected) { + const std::string& expected) { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); BOOST_REQUIRE_EQUAL(expected, featureData.print()); @@ -1416,7 +1415,7 @@ class CDistinctStringsTestFixture : public CTestFixture { // Similar helper for the info-content feature data. static void verifyInfoContentFeature(const CUniqueStringFeatureData& data, - const std::string& expected) { + const std::string& expected) { SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); BOOST_REQUIRE_EQUAL(expected, featureData.print()); @@ -1459,7 +1458,7 @@ class CDistinctStringsTestFixture : public CTestFixture { data.populateDistinctCountFeatureData(featureData); BOOST_REQUIRE_EQUAL(std::max(static_cast(3), static_cast(i)), - featureData.s_Count); + featureData.s_Count); } } @@ -1551,9 +1550,12 @@ class CDistinctStringsTestFixture : public CTestFixture { data.insert(ss.str(), influencers); SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); - BOOST_TEST_REQUIRE((featureData.s_Count - 12) >= std::max(static_cast(3), + BOOST_TEST_REQUIRE((featureData.s_Count - 12) >= + std::max(static_cast(3), static_cast(i))); - BOOST_TEST_REQUIRE((featureData.s_Count - 12) <= std::max(static_cast(3), static_cast(i)) * 3); + BOOST_TEST_REQUIRE( + (featureData.s_Count - 12) <= + std::max(static_cast(3), static_cast(i)) * 3); } } @@ -1618,7 +1620,7 @@ class CDistinctStringsTestFixture : public CTestFixture { data.populateInfoContentFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { std::ranges::sort(featureData.s_InfluenceValues[i], - maths::common::COrderings::SFirstLess()); + maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("18, [[(inf1, ([16], 1)), (inf2, ([16], 1))], [(inf_v2, ([12], 1)), (inf_v3, ([16], 1))]]"), featureData.print()); @@ -1657,10 +1659,14 @@ class CDistinctStringsTestFixture : public CTestFixture { testPersistence(params, gatherer, model_t::E_EventRate); // Add data (some out-of-order) for distinct strings. - addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", "stringOne", "inf1"); - addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", "stringTwo", "inf2"); - addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", "stringThree", "inf3"); - addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", "stringFour", "inf1"); + addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", + "stringOne", "inf1"); + addArrival(gatherer, m_ResourceMonitor, time - (2 * bucketLength), "p", + "stringTwo", "inf2"); + addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", + "stringThree", "inf3"); + addArrival(gatherer, m_ResourceMonitor, time - (1 * bucketLength), "p", + "stringFour", "inf1"); addArrival(gatherer, m_ResourceMonitor, time, "p", "stringFive", "inf2"); addArrival(gatherer, m_ResourceMonitor, time, "p", "stringSix", "inf3"); testPersistence(params, gatherer, model_t::E_EventRate); @@ -1715,6 +1721,7 @@ class CDiurnalTestFixture : public CTestFixture { .attributeFieldName("att") .build(); } + if (!isPopulation) { return builder.personFieldName("person").build(); } @@ -1752,9 +1759,7 @@ class CDiurnalTestFixture : public CTestFixture { // - TFeatureSizeFeatureDataPrVecPrVec for tests by person, // - TFeatureSizeSizePrFeatureDataPrVecPrVec for tests over person. template - void verifyFeatureData(const CDataGatherer& gatherer, - core_t::TTime time, - std::uint64_t expectedCount) { + void verifyFeatureData(const CDataGatherer& gatherer, core_t::TTime time, std::uint64_t expectedCount) { FeatureDataT featureData; gatherer.featureData(time, BUCKET_LENGTH, featureData); BOOST_REQUIRE_EQUAL(1, featureData.size()); @@ -1797,8 +1802,7 @@ class CDiurnalTestFixture : public CTestFixture { // Arrival 6: time + 300, expected additional count of 150. addArrivalHelper(gatherer, time + 300, useAttribute); - verifyFeatureData(gatherer, time, - computeExpected(time, 150, isDay)); + verifyFeatureData(gatherer, time, computeExpected(time, 150, isDay)); // Check latency: go back two buckets. time -= BUCKET_LENGTH * 2; @@ -1807,12 +1811,13 @@ class CDiurnalTestFixture : public CTestFixture { time += BUCKET_LENGTH; addArrivalHelper(gatherer, time + 400, useAttribute); - verifyFeatureData(gatherer, time, - computeExpected(time, 200, isDay)); + verifyFeatureData(gatherer, time, computeExpected(time, 200, isDay)); } // Verify summary information for the gatherer. - static void verifyGathererSummary(const CDataGatherer& gatherer, bool isPopulation, bool useAttribute) { + static void verifyGathererSummary(const CDataGatherer& gatherer, + bool isPopulation, + bool useAttribute) { if (isPopulation) { if (useAttribute) { BOOST_REQUIRE_EQUAL(1, gatherer.numberActivePeople()); @@ -1832,10 +1837,12 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_day by person LOG_DEBUG(<< "Testing time_of_day by person"); SModelParams const params = createParams(); - TFeatureVec const features{ model_t::E_IndividualTimeOfDayByBucketAndPerson }; + TFeatureVec const features{model_t::E_IndividualTimeOfDayByBucketAndPerson}; CDataGatherer gatherer = createGatherer(features, params, START_TIME, false); - verifyGathererFeatures(gatherer, features, model_t::E_IndividualTimeOfDayByBucketAndPerson, false); - runTestSequence(gatherer, /*isDay=*/true, /*useAttribute=*/false); + verifyGathererFeatures(gatherer, features, + model_t::E_IndividualTimeOfDayByBucketAndPerson, false); + runTestSequence(gatherer, /*isDay=*/true, + /*useAttribute=*/false); verifyGathererSummary(gatherer, false, false); testPersistence(params, gatherer, model_t::E_EventRate); } @@ -1843,10 +1850,12 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_week by person LOG_DEBUG(<< "Testing time_of_week by person"); SModelParams const params = createParams(); - TFeatureVec const features{ model_t::E_IndividualTimeOfWeekByBucketAndPerson }; + TFeatureVec const features{model_t::E_IndividualTimeOfWeekByBucketAndPerson}; CDataGatherer gatherer = createGatherer(features, params, START_TIME, false); - verifyGathererFeatures(gatherer, features, model_t::E_IndividualTimeOfWeekByBucketAndPerson, false); - runTestSequence(gatherer, /*isDay=*/false, /*useAttribute=*/false); + verifyGathererFeatures(gatherer, features, + model_t::E_IndividualTimeOfWeekByBucketAndPerson, false); + runTestSequence( + gatherer, /*isDay=*/false, /*useAttribute=*/false); verifyGathererSummary(gatherer, false, false); testPersistence(params, gatherer, model_t::E_EventRate); } @@ -1854,10 +1863,13 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_week over person (with attribute) LOG_DEBUG(<< "Testing time_of_week over person"); SModelParams const params = createParams(); - TFeatureVec const features{ model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute }; - CDataGatherer gatherer = createGatherer(features, params, START_TIME, true, /*useAttribute=*/true); - verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute, true); - runTestSequence(gatherer, /*isDay=*/false, /*useAttribute=*/true); + TFeatureVec const features{model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute}; + CDataGatherer gatherer = createGatherer(features, params, START_TIME, + true, /*useAttribute=*/true); + verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfWeekByBucketPersonAndAttribute, + true); + runTestSequence( + gatherer, /*isDay=*/false, /*useAttribute=*/true); verifyGathererSummary(gatherer, true, true); testPersistence(params, gatherer, model_t::E_EventRate); } @@ -1865,10 +1877,13 @@ BOOST_FIXTURE_TEST_CASE(testDiurnalFeatures, CDiurnalTestFixture) { // Test: time_of_day over person (with attribute) LOG_DEBUG(<< "Testing time_of_day over person"); SModelParams const params = createParams(); - TFeatureVec const features{ model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute }; - CDataGatherer gatherer = createGatherer(features, params, START_TIME, true, /*useAttribute=*/true); - verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute, true); - runTestSequence(gatherer, /*isDay=*/true, /*useAttribute=*/true); + TFeatureVec const features{model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute}; + CDataGatherer gatherer = createGatherer(features, params, START_TIME, + true, /*useAttribute=*/true); + verifyGathererFeatures(gatherer, features, model_t::E_PopulationTimeOfDayByBucketPersonAndAttribute, + true); + runTestSequence( + gatherer, /*isDay=*/true, /*useAttribute=*/true); verifyGathererSummary(gatherer, true, true); testPersistence(params, gatherer, model_t::E_EventRate); } diff --git a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc index 91c7f8775..42270fb7f 100644 --- a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc +++ b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc @@ -477,9 +477,9 @@ BOOST_FIXTURE_TEST_CASE(testCompressedLength, CTestFixture) { features.push_back(model_t::E_PopulationInfoContentByBucketPersonAndAttribute); SModelParams constparams(bucketLength); CDataGatherer dataGatherer = CDataGathererBuilder(model_t::E_PopulationEventRate, - features, params, searchKey, startTime) - .valueFieldName("value") - .build(); + features, params, searchKey, startTime) + .valueFieldName("value") + .build(); core_t::TTime time = startTime; for (std::size_t i = 0; i < numberBuckets; ++i, time += bucketLength) { TMessageVec messages; @@ -809,6 +809,7 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { features.push_back(model_t::E_PopulationUniquePersonCountByAttribute); SModelParams const params(bucketLength); CDataGatherer origDataGatherer = + CDataGathererBuilder(model_t::E_PopulationEventRate, features, params, searchKey, startTime) .build(); @@ -831,6 +832,7 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { features.push_back(model_t::E_PopulationInfoContentByBucketPersonAndAttribute); SModelParams const params(bucketLength); CDataGatherer dataGatherer = + CDataGathererBuilder(model_t::E_PopulationEventRate, features, params, searchKey, startTime) .valueFieldName("value") diff --git a/lib/model/unittest/CMetricDataGathererTest.cc b/lib/model/unittest/CMetricDataGathererTest.cc index 1dc81d8ae..9f1a88a6a 100644 --- a/lib/model/unittest/CMetricDataGathererTest.cc +++ b/lib/model/unittest/CMetricDataGathererTest.cc @@ -689,6 +689,7 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { gatherer.recyclePeople(peopleToRemove); CDataGatherer expectedGatherer = + CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) .build(); BOOST_REQUIRE_EQUAL(0, addPerson("p3", expectedGatherer, m_ResourceMonitor)); @@ -721,6 +722,7 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { gatherer.recyclePeople(peopleToRemove); CDataGatherer expectedGatherer = + CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) .build(); @@ -751,6 +753,7 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { gatherer.recyclePeople(peopleToRemove); CDataGatherer const expectedGatherer = + CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) .build(); @@ -1128,7 +1131,10 @@ BOOST_FIXTURE_TEST_CASE(testResetBucketGivenMultipleSeries, CTestFixture) { features.push_back(model_t::E_IndividualMinByPerson); features.push_back(model_t::E_IndividualMaxByPerson); features.push_back(model_t::E_IndividualSumByBucketAndPerson); - CDataGatherer gatherer = CDataGathererBuilder(model_t::E_Metric,features, params, KEY, startTime).sampleCountOverride(2U).build(); + CDataGatherer gatherer = + CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) + .sampleCountOverride(2U) + .build(); addPerson("p1", gatherer, m_ResourceMonitor); addPerson("p2", gatherer, m_ResourceMonitor); addPerson("p3", gatherer, m_ResourceMonitor); diff --git a/lib/model/unittest/CMetricPopulationDataGathererTest.cc b/lib/model/unittest/CMetricPopulationDataGathererTest.cc index b5e6279e2..738c49f5d 100644 --- a/lib/model/unittest/CMetricPopulationDataGathererTest.cc +++ b/lib/model/unittest/CMetricPopulationDataGathererTest.cc @@ -998,6 +998,7 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { features.push_back(model_t::E_PopulationMaxByPersonAndAttribute); features.push_back(model_t::E_PopulationHighSumByBucketPersonAndAttribute); CDataGatherer origDataGatherer = + CDataGathererBuilder(model_t::E_PopulationMetric, features, params, searchKey, startTime) .build(); @@ -1030,8 +1031,8 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { core::CJsonStateRestoreTraverser traverser(origJsonStrm); auto bucketInitData = CBucketGatherer::SBucketGathererInitData::emptyData(); CDataGatherer restoredDataGatherer(model_t::E_PopulationMetric, - model_t::E_None, params, - EMPTY_STRING, searchKey, bucketInitData, traverser); + model_t::E_None, params, EMPTY_STRING, + searchKey, bucketInitData, traverser); // The JSON representation of the new data gatherer should be the same as the // original diff --git a/lib/model/unittest/ModelTestHelpers.h b/lib/model/unittest/ModelTestHelpers.h index 5fbd4cdce..e7f0ab9fa 100644 --- a/lib/model/unittest/ModelTestHelpers.h +++ b/lib/model/unittest/ModelTestHelpers.h @@ -46,16 +46,10 @@ static void testPersistence(const SModelParams& params, core::CJsonStateRestoreTraverser traverser(origJsonStrm); CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - EMPTY_STRING, - EMPTY_STRING, - EMPTY_STRING, - EMPTY_STRING, - {}, - 0, - 0}; + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; - CDataGatherer restoredGatherer(category, model_t::E_None, params, - EMPTY_STRING, KEY, bucketGathererInitData, traverser); + CDataGatherer restoredGatherer(category, model_t::E_None, params, EMPTY_STRING, + KEY, bucketGathererInitData, traverser); BOOST_REQUIRE_EQUAL(origGatherer.checksum(), restoredGatherer.checksum()); @@ -112,9 +106,8 @@ class CDataGathererBuilder { m_InfluenceFieldNames, m_StartTime, static_cast(m_SampleCountOverride)}; - return {m_GathererType, m_SummaryMode, m_Params, - m_PartitionFieldValue, m_SearchKey, - m_Features, + return {m_GathererType, m_SummaryMode, m_Params, + m_PartitionFieldValue, m_SearchKey, m_Features, bucketGathererInitData}; } @@ -141,8 +134,8 @@ class CDataGathererBuilder { m_StartTime, static_cast(m_SampleCountOverride)}; return std::make_shared(m_GathererType, m_SummaryMode, m_Params, - m_PartitionFieldValue, m_SearchKey, m_Features, - bucketGathererInitData); + m_PartitionFieldValue, m_SearchKey, + m_Features, bucketGathererInitData); } CDataGathererBuilder& partitionFieldValue(const std::string& partitionFieldValue) { From d0dfcb974fbc3a319e3308920bd24e11a9859d3c Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 27 Mar 2025 16:44:41 +0100 Subject: [PATCH 07/21] Extend influencer counts if the resource monitor allows --- include/model/CBucketGatherer.h | 4 +++- lib/model/CBucketGatherer.cc | 19 ++++++++++--------- lib/model/CEventRateBucketGatherer.cc | 5 ++--- lib/model/CMetricBucketGatherer.cc | 5 ++--- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 1beaaebca..0ba6f86eb 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -168,7 +168,7 @@ class MODEL_EXPORT CBucketGatherer { //! to gather data. //! \param[in] numberInfluencers The number of result influencers //! for which to gather data. - CBucketGatherer(CDataGatherer& dataGatherer, core_t::TTime startTime, std::size_t numberInfluencers); + CBucketGatherer(CDataGatherer& dataGatherer, const SBucketGathererInitData& bucketGathererInitData); //! Create a copy that will result in the same persisted state as the //! original. This is effectively a copy constructor that creates a @@ -474,6 +474,8 @@ class MODEL_EXPORT CBucketGatherer { //! The influencing field value counts per person and/or attribute. TSizeSizePrOptionalStrPrUInt64UMapVecQueue m_InfluencerCounts; + + TOptionalResourceMonitorCRef m_ResourceMonitor; }; } } diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index 7a987ec6a..c6479d597 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -9,6 +9,7 @@ * limitation. */ +#include "model/CResourceMonitor.h" #include #include @@ -202,21 +203,21 @@ const std::string CBucketGatherer::EVENTRATE_BUCKET_GATHERER_TAG("a"); const std::string CBucketGatherer::METRIC_BUCKET_GATHERER_TAG("b"); CBucketGatherer::CBucketGatherer(CDataGatherer& dataGatherer, - core_t::TTime startTime, - std::size_t numberInfluencers) - : m_DataGatherer(dataGatherer), m_EarliestTime(startTime), m_BucketStart(startTime), + const SBucketGathererInitData& initData) + : m_DataGatherer(dataGatherer), m_EarliestTime(initData.s_StartTime), m_BucketStart(initData.s_StartTime), m_PersonAttributeCounts(dataGatherer.params().s_LatencyBuckets, dataGatherer.params().s_BucketLength, - startTime, + initData.s_StartTime, TSizeSizePrUInt64UMap(1)), m_PersonAttributeExplicitNulls(dataGatherer.params().s_LatencyBuckets, dataGatherer.params().s_BucketLength, - startTime, + initData.s_StartTime, TSizeSizePrUSet(1)), m_InfluencerCounts(dataGatherer.params().s_LatencyBuckets + 3, dataGatherer.params().s_BucketLength, - startTime, - TSizeSizePrOptionalStrPrUInt64UMapVec(numberInfluencers)) { + initData.s_StartTime, + TSizeSizePrOptionalStrPrUInt64UMapVec(initData.s_InfluenceFieldNames.size())), +m_ResourceMonitor(initData.s_ResourceMonitor){ } CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& other) @@ -224,7 +225,7 @@ CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& o m_EarliestTime(other.m_EarliestTime), m_BucketStart(other.m_BucketStart), m_PersonAttributeCounts(other.m_PersonAttributeCounts), m_PersonAttributeExplicitNulls(other.m_PersonAttributeExplicitNulls), - m_InfluencerCounts(other.m_InfluencerCounts) { + m_InfluencerCounts(other.m_InfluencerCounts), m_ResourceMonitor(other.m_ResourceMonitor) { if (!isForPersistence) { LOG_ABORT(<< "This constructor only creates clones for persistence"); } @@ -293,7 +294,7 @@ bool CBucketGatherer::addEventData(CEventData& data) { if (influence) { const std::string& inf = *influence; canonicalInfluences[i] = inf; - if (count > 0) { + if (count > 0 && m_ResourceMonitor && m_ResourceMonitor->get().areAllocationsAllowed()) { influencerCounts[i] .emplace(boost::unordered::piecewise_construct, boost::make_tuple(pidCid, inf), diff --git a/lib/model/CEventRateBucketGatherer.cc b/lib/model/CEventRateBucketGatherer.cc index a01aa9479..0ee0e97bf 100644 --- a/lib/model/CEventRateBucketGatherer.cc +++ b/lib/model/CEventRateBucketGatherer.cc @@ -729,8 +729,7 @@ void registerMemoryCallbacks() { CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData) : CBucketGatherer(dataGatherer, - initData.s_StartTime, - initData.s_InfluenceFieldNames.size()), + initData), m_BeginInfluencingFields(0), m_BeginValueField(0), m_BeginSummaryFields(0) { this->initializeFieldNames(initData); this->initializeFeatureData(); @@ -739,7 +738,7 @@ CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) - : CBucketGatherer(dataGatherer, 0, initData.s_InfluenceFieldNames.size()), + : CBucketGatherer(dataGatherer, initData), m_BeginInfluencingFields(0), m_BeginValueField(0), m_BeginSummaryFields(0) { this->initializeFieldNames(initData); if (traverser.traverseSubLevel(std::bind(&CEventRateBucketGatherer::acceptRestoreTraverser, diff --git a/lib/model/CMetricBucketGatherer.cc b/lib/model/CMetricBucketGatherer.cc index 06439cfef..ffba2c622 100644 --- a/lib/model/CMetricBucketGatherer.cc +++ b/lib/model/CMetricBucketGatherer.cc @@ -927,8 +927,7 @@ struct SReleaseMemory { CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData) : CBucketGatherer(dataGatherer, - initData.s_StartTime, - initData.s_InfluenceFieldNames.size()), + initData), m_ValueFieldName(initData.s_ValueFieldName), m_BeginInfluencingFields(0), m_BeginValueFields(0) { this->initializeFieldNamesPart1(initData); @@ -939,7 +938,7 @@ CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) - : CBucketGatherer(dataGatherer, 0, initData.s_InfluenceFieldNames.size()), + : CBucketGatherer(dataGatherer, initData), m_ValueFieldName(initData.s_ValueFieldName), m_BeginValueFields(0) { this->initializeFieldNamesPart1(initData); if (traverser.traverseSubLevel(std::bind(&CMetricBucketGatherer::acceptRestoreTraverser, From 3cda9ce26dcd1af6b716d39ee199fa1ac979ee13 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Tue, 8 Apr 2025 15:24:13 +0200 Subject: [PATCH 08/21] update clang-tidy file --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index 146d35bce..9459552c1 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -23,6 +23,7 @@ Checks: > -modernize-use-trailing-return-type, -modernize-concat-nested-namespaces, -modernize-use-nodiscard, + -modernize-use-ranges, performance-*, From c3e9089cc1a5c87ceb50ff722346f23f58828c16 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Tue, 8 Apr 2025 15:54:02 +0200 Subject: [PATCH 09/21] fix merge errors --- lib/model/unittest/CDetectionRuleTest.cc | 67 +++++++++---------- .../unittest/CEventRateDataGathererTest.cc | 7 +- .../CEventRatePopulationDataGathererTest.cc | 2 +- .../unittest/CHierarchicalResultsTest.cc | 11 ++- lib/model/unittest/CMetricDataGathererTest.cc | 6 +- lib/model/unittest/ModelTestHelpers.h | 9 --- 6 files changed, 45 insertions(+), 57 deletions(-) diff --git a/lib/model/unittest/CDetectionRuleTest.cc b/lib/model/unittest/CDetectionRuleTest.cc index 19f643cad..8821d2573 100644 --- a/lib/model/unittest/CDetectionRuleTest.cc +++ b/lib/model/unittest/CDetectionRuleTest.cc @@ -794,40 +794,39 @@ BOOST_FIXTURE_TEST_CASE(testRuleActions, CTestFixture) { 0, 0, 100)); } -TMockModelPtr initializeModel(ml::model::CResourceMonitor& resourceMonitor) { - core_t::TTime bucketLength{600}; - model::SModelParams params{bucketLength}; - model::CSearchKey key; - model_t::TFeatureVec features; - // Initialize mock model - model::CAnomalyDetectorModel::TDataGathererPtr gatherer; - - features.assign(1, model_t::E_IndividualSumByBucketAndPerson); - - gatherer = std::make_shared( - model_t::analysisCategory(features[0]), model_t::E_None, params, EMPTY_STRING, - EMPTY_STRING, "p", EMPTY_STRING, EMPTY_STRING, TStrVec{}, key, features, 0, 0); - - std::string person("p1"); - bool addedPerson{false}; - gatherer->addPerson(person, resourceMonitor, addedPerson); - - TMockModelPtr model{new model::CMockModel( - params, gatherer, {/* we don't care about influence */})}; - - maths::time_series::CTimeSeriesDecomposition trend; - maths::common::CNormalMeanPrecConjugate prior{ - maths::common::CNormalMeanPrecConjugate::nonInformativePrior(maths_t::E_ContinuousData)}; - maths::common::CModelParams timeSeriesModelParams{ - bucketLength, 1.0, 0.001, 0.2, 6 * core::constants::HOUR, 24 * core::constants::HOUR}; - std::unique_ptr timeSeriesModel = - std::make_unique( - timeSeriesModelParams, 0, trend, prior); - model::CMockModel::TMathsModelUPtrVec models; - models.emplace_back(std::move(timeSeriesModel)); - model->mockTimeSeriesModels(std::move(models)); - return model; -} +// TMockModelPtr initializeModel(ml::model::CResourceMonitor& resourceMonitor) { +// core_t::TTime bucketLength{600}; +// model::SModelParams params{bucketLength}; +// model::CSearchKey key; +// model_t::TFeatureVec features; +// // Initialize mock model +// model::CAnomalyDetectorModel::TDataGathererPtr gatherer; +// +// features.assign(1, model_t::E_IndividualSumByBucketAndPerson); +// +// gatherer = CDataGathererBuilder(model_t::analysisCategory(features[0]), features, params, key, 0) +// .personFieldName("p") +// .buildSharedPtr(); +// std::string person("p1"); +// bool addedPerson{false}; +// gatherer->addPerson(person, resourceMonitor, addedPerson); +// +// TMockModelPtr model{new model::CMockModel( +// params, gatherer, {/* we don't care about influence */})}; +// +// maths::time_series::CTimeSeriesDecomposition trend; +// maths::common::CNormalMeanPrecConjugate prior{ +// maths::common::CNormalMeanPrecConjugate::nonInformativePrior(maths_t::E_ContinuousData)}; +// maths::common::CModelParams timeSeriesModelParams{ +// bucketLength, 1.0, 0.001, 0.2, 6 * core::constants::HOUR, 24 * core::constants::HOUR}; +// std::unique_ptr timeSeriesModel = +// std::make_unique( +// timeSeriesModelParams, 0, trend, prior); +// model::CMockModel::TMathsModelUPtrVec models; +// models.emplace_back(std::move(timeSeriesModel)); +// model->mockTimeSeriesModels(std::move(models)); +// return model; +// } BOOST_FIXTURE_TEST_CASE(testRuleTimeShiftShouldShiftTimeSeriesModelState, CTestFixture) { diff --git a/lib/model/unittest/CEventRateDataGathererTest.cc b/lib/model/unittest/CEventRateDataGathererTest.cc index 96b15c590..bb32eba5e 100644 --- a/lib/model/unittest/CEventRateDataGathererTest.cc +++ b/lib/model/unittest/CEventRateDataGathererTest.cc @@ -834,7 +834,6 @@ BOOST_FIXTURE_TEST_CASE(testSingleSeriesOutOfOrderFinalResult, CTestFixture) { }; const std::array expectedPersonCounts = { - constexpr std::array expectedPersonCounts = { std::string("[(0, 6)]"), std::string("[(0, 3)]"), std::string("[(0, 2)]"), std::string("[(0, 0)]"), std::string("[(0, 3)]")}; @@ -1489,7 +1488,7 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); - std::ranges::sort(featureData.s_InfluenceValues[0], + std::sort(featureData.s_InfluenceValues[0].begin(), featureData.s_InfluenceValues[0].end(), maths::common::COrderings::SFirstLess()); BOOST_REQUIRE_EQUAL(std::string("3, [[(inf1, ([2], 1)), (inf2, ([2], 1)), (inf3, ([1], 1))]]"), featureData.print()); @@ -1523,7 +1522,7 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { - std::ranges::sort(featureData.s_InfluenceValues[i], + std::sort(featureData.s_InfluenceValues[i].begin(), featureData.s_InfluenceValues[i].end(), maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("3, [[(inf1, ([2], 1)), (inf2, ([2], 1))], [(inf_v2, ([1], 1)), (inf_v3, ([2], 1))]]"), @@ -1619,7 +1618,7 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { - std::ranges::sort(featureData.s_InfluenceValues[i], + std::sort(featureData.s_InfluenceValues[i].begin(), featureData.s_InfluenceValues[i].end(), maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("18, [[(inf1, ([16], 1)), (inf2, ([16], 1))], [(inf_v2, ([12], 1)), (inf_v3, ([16], 1))]]"), diff --git a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc index 42270fb7f..c3f0fc483 100644 --- a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc +++ b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc @@ -475,7 +475,7 @@ BOOST_FIXTURE_TEST_CASE(testCompressedLength, CTestFixture) { CDataGatherer::TFeatureVec features; features.push_back(model_t::E_PopulationInfoContentByBucketPersonAndAttribute); - SModelParams constparams(bucketLength); + SModelParams const params(bucketLength); CDataGatherer dataGatherer = CDataGathererBuilder(model_t::E_PopulationEventRate, features, params, searchKey, startTime) .valueFieldName("value") diff --git a/lib/model/unittest/CHierarchicalResultsTest.cc b/lib/model/unittest/CHierarchicalResultsTest.cc index a6f4c98f9..4b54244cb 100644 --- a/lib/model/unittest/CHierarchicalResultsTest.cc +++ b/lib/model/unittest/CHierarchicalResultsTest.cc @@ -1447,12 +1447,11 @@ BOOST_AUTO_TEST_CASE(testWriter) { auto interimBucketCorrector = std::make_shared(modelConfig.bucketLength()); model::CSearchKey key; - model::CAnomalyDetectorModel::TDataGathererPtr dataGatherer( - std::make_shared( - model_t::E_EventRate, model_t::E_None, params, EMPTY_STRING, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, TStrVec{}, key, - model_t::TFeatureVec{model_t::E_IndividualCountByBucketAndPerson}, - modelConfig.bucketLength(), 0)); + auto dataGatherer = + model::CDataGathererBuilder(model_t::E_EventRate, + {model_t::E_IndividualCountByBucketAndPerson}, + params, key, modelConfig.bucketLength()) + .buildSharedPtr(); model::CEventData dummy; dataGatherer->addArrival(TStrCPtrVec(1, &EMPTY_STRING), dummy, resourceMonitor); dummy.clear(); diff --git a/lib/model/unittest/CMetricDataGathererTest.cc b/lib/model/unittest/CMetricDataGathererTest.cc index 9f1a88a6a..09e76198a 100644 --- a/lib/model/unittest/CMetricDataGathererTest.cc +++ b/lib/model/unittest/CMetricDataGathererTest.cc @@ -1607,9 +1607,9 @@ BOOST_FIXTURE_TEST_CASE(testMultivariate, CTestFixture) { { TFeatureVec features; features.push_back(model_t::E_IndividualMeanLatLongByPerson); - CDataGatherer gatherer(model_t::E_Metric, model_t::E_None, params, - EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, - EMPTY_STRING, {}, KEY, features, startTime, 0); + CDataGatherer gatherer = CDataGathererBuilder(model_t::E_Metric, features, + params, KEY, startTime) + .build(); BOOST_REQUIRE_EQUAL(0, addPerson("p", gatherer, m_ResourceMonitor)); TTimeDoubleDoubleTupleVecVec buckets; diff --git a/lib/model/unittest/ModelTestHelpers.h b/lib/model/unittest/ModelTestHelpers.h index e7f0ab9fa..3898cb094 100644 --- a/lib/model/unittest/ModelTestHelpers.h +++ b/lib/model/unittest/ModelTestHelpers.h @@ -110,15 +110,6 @@ class CDataGathererBuilder { m_PartitionFieldValue, m_SearchKey, m_Features, bucketGathererInitData}; } - - std::shared_ptr buildSharedPtr() const { - return std::make_shared( - m_GathererType, m_SummaryMode, m_Params, m_SummaryCountFieldName, - m_PartitionFieldValue, m_PersonFieldName, m_AttributeFieldName, - m_ValueFieldName, m_InfluenceFieldNames, m_SearchKey, m_Features, - m_StartTime, m_SampleCountOverride); - } - CDataGathererBuilder& partitionFieldValue(std::string_view partitionFieldValue) { m_PartitionFieldValue = partitionFieldValue; return *this; From 94f2efe62a17363395ce10569e841d5645a63cb7 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Tue, 8 Apr 2025 15:55:20 +0200 Subject: [PATCH 10/21] fix merge errors --- include/model/CBucketGatherer.h | 3 ++- lib/model/CBucketGatherer.cc | 14 +++++++++----- lib/model/CEventRateBucketGatherer.cc | 9 ++++----- lib/model/CMetricBucketGatherer.cc | 3 +-- lib/model/unittest/CEventRateDataGathererTest.cc | 15 +++++++++------ lib/model/unittest/CMetricDataGathererTest.cc | 2 +- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 0ba6f86eb..89383da98 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -168,7 +168,8 @@ class MODEL_EXPORT CBucketGatherer { //! to gather data. //! \param[in] numberInfluencers The number of result influencers //! for which to gather data. - CBucketGatherer(CDataGatherer& dataGatherer, const SBucketGathererInitData& bucketGathererInitData); + CBucketGatherer(CDataGatherer& dataGatherer, + const SBucketGathererInitData& bucketGathererInitData); //! Create a copy that will result in the same persisted state as the //! original. This is effectively a copy constructor that creates a diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index c6479d597..40a396f79 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -204,7 +204,8 @@ const std::string CBucketGatherer::METRIC_BUCKET_GATHERER_TAG("b"); CBucketGatherer::CBucketGatherer(CDataGatherer& dataGatherer, const SBucketGathererInitData& initData) - : m_DataGatherer(dataGatherer), m_EarliestTime(initData.s_StartTime), m_BucketStart(initData.s_StartTime), + : m_DataGatherer(dataGatherer), m_EarliestTime(initData.s_StartTime), + m_BucketStart(initData.s_StartTime), m_PersonAttributeCounts(dataGatherer.params().s_LatencyBuckets, dataGatherer.params().s_BucketLength, initData.s_StartTime, @@ -216,8 +217,9 @@ CBucketGatherer::CBucketGatherer(CDataGatherer& dataGatherer, m_InfluencerCounts(dataGatherer.params().s_LatencyBuckets + 3, dataGatherer.params().s_BucketLength, initData.s_StartTime, - TSizeSizePrOptionalStrPrUInt64UMapVec(initData.s_InfluenceFieldNames.size())), -m_ResourceMonitor(initData.s_ResourceMonitor){ + TSizeSizePrOptionalStrPrUInt64UMapVec( + initData.s_InfluenceFieldNames.size())), + m_ResourceMonitor(initData.s_ResourceMonitor) { } CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& other) @@ -225,7 +227,8 @@ CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& o m_EarliestTime(other.m_EarliestTime), m_BucketStart(other.m_BucketStart), m_PersonAttributeCounts(other.m_PersonAttributeCounts), m_PersonAttributeExplicitNulls(other.m_PersonAttributeExplicitNulls), - m_InfluencerCounts(other.m_InfluencerCounts), m_ResourceMonitor(other.m_ResourceMonitor) { + m_InfluencerCounts(other.m_InfluencerCounts), + m_ResourceMonitor(other.m_ResourceMonitor) { if (!isForPersistence) { LOG_ABORT(<< "This constructor only creates clones for persistence"); } @@ -294,7 +297,8 @@ bool CBucketGatherer::addEventData(CEventData& data) { if (influence) { const std::string& inf = *influence; canonicalInfluences[i] = inf; - if (count > 0 && m_ResourceMonitor && m_ResourceMonitor->get().areAllocationsAllowed()) { + if (count > 0 && m_ResourceMonitor && + m_ResourceMonitor->get().areAllocationsAllowed()) { influencerCounts[i] .emplace(boost::unordered::piecewise_construct, boost::make_tuple(pidCid, inf), diff --git a/lib/model/CEventRateBucketGatherer.cc b/lib/model/CEventRateBucketGatherer.cc index 0ee0e97bf..aa3ab46ab 100644 --- a/lib/model/CEventRateBucketGatherer.cc +++ b/lib/model/CEventRateBucketGatherer.cc @@ -728,9 +728,8 @@ void registerMemoryCallbacks() { CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData) - : CBucketGatherer(dataGatherer, - initData), - m_BeginInfluencingFields(0), m_BeginValueField(0), m_BeginSummaryFields(0) { + : CBucketGatherer(dataGatherer, initData), m_BeginInfluencingFields(0), + m_BeginValueField(0), m_BeginSummaryFields(0) { this->initializeFieldNames(initData); this->initializeFeatureData(); } @@ -738,8 +737,8 @@ CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) - : CBucketGatherer(dataGatherer, initData), - m_BeginInfluencingFields(0), m_BeginValueField(0), m_BeginSummaryFields(0) { + : CBucketGatherer(dataGatherer, initData), m_BeginInfluencingFields(0), + m_BeginValueField(0), m_BeginSummaryFields(0) { this->initializeFieldNames(initData); if (traverser.traverseSubLevel(std::bind(&CEventRateBucketGatherer::acceptRestoreTraverser, this, std::placeholders::_1)) == false) { diff --git a/lib/model/CMetricBucketGatherer.cc b/lib/model/CMetricBucketGatherer.cc index ffba2c622..830787364 100644 --- a/lib/model/CMetricBucketGatherer.cc +++ b/lib/model/CMetricBucketGatherer.cc @@ -926,8 +926,7 @@ struct SReleaseMemory { CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, const CBucketGatherer::SBucketGathererInitData& initData) - : CBucketGatherer(dataGatherer, - initData), + : CBucketGatherer(dataGatherer, initData), m_ValueFieldName(initData.s_ValueFieldName), m_BeginInfluencingFields(0), m_BeginValueFields(0) { this->initializeFieldNamesPart1(initData); diff --git a/lib/model/unittest/CEventRateDataGathererTest.cc b/lib/model/unittest/CEventRateDataGathererTest.cc index bb32eba5e..859d0e204 100644 --- a/lib/model/unittest/CEventRateDataGathererTest.cc +++ b/lib/model/unittest/CEventRateDataGathererTest.cc @@ -1488,8 +1488,9 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); - std::sort(featureData.s_InfluenceValues[0].begin(), featureData.s_InfluenceValues[0].end(), - maths::common::COrderings::SFirstLess()); + std::sort(featureData.s_InfluenceValues[0].begin(), + featureData.s_InfluenceValues[0].end(), + maths::common::COrderings::SFirstLess()); BOOST_REQUIRE_EQUAL(std::string("3, [[(inf1, ([2], 1)), (inf2, ([2], 1)), (inf3, ([1], 1))]]"), featureData.print()); } @@ -1522,8 +1523,9 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateDistinctCountFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { - std::sort(featureData.s_InfluenceValues[i].begin(), featureData.s_InfluenceValues[i].end(), - maths::common::COrderings::SFirstLess()); + std::sort(featureData.s_InfluenceValues[i].begin(), + featureData.s_InfluenceValues[i].end(), + maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("3, [[(inf1, ([2], 1)), (inf2, ([2], 1))], [(inf_v2, ([1], 1)), (inf_v3, ([2], 1))]]"), featureData.print()); @@ -1618,8 +1620,9 @@ class CDistinctStringsTestFixture : public CTestFixture { SEventRateFeatureData featureData(0); data.populateInfoContentFeatureData(featureData); for (std::size_t i = 0; i < 2; i++) { - std::sort(featureData.s_InfluenceValues[i].begin(), featureData.s_InfluenceValues[i].end(), - maths::common::COrderings::SFirstLess()); + std::sort(featureData.s_InfluenceValues[i].begin(), + featureData.s_InfluenceValues[i].end(), + maths::common::COrderings::SFirstLess()); } BOOST_REQUIRE_EQUAL(std::string("18, [[(inf1, ([16], 1)), (inf2, ([16], 1))], [(inf_v2, ([12], 1)), (inf_v3, ([16], 1))]]"), featureData.print()); diff --git a/lib/model/unittest/CMetricDataGathererTest.cc b/lib/model/unittest/CMetricDataGathererTest.cc index 09e76198a..fc9e4472a 100644 --- a/lib/model/unittest/CMetricDataGathererTest.cc +++ b/lib/model/unittest/CMetricDataGathererTest.cc @@ -1608,7 +1608,7 @@ BOOST_FIXTURE_TEST_CASE(testMultivariate, CTestFixture) { TFeatureVec features; features.push_back(model_t::E_IndividualMeanLatLongByPerson); CDataGatherer gatherer = CDataGathererBuilder(model_t::E_Metric, features, - params, KEY, startTime) + params, KEY, startTime) .build(); BOOST_REQUIRE_EQUAL(0, addPerson("p", gatherer, m_ResourceMonitor)); From 9327ce8aebdf556f816737a3c966451951d77c5c Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 14:31:06 +0200 Subject: [PATCH 11/21] add documentation --- docs/CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index 75117ca06..cc0ec094f 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -33,6 +33,7 @@ === Enhancements * Track memory used in the hierarchical results normalizer. (See {ml-pull}2831[#2831].) +* Improve adherence to memory limits in the hard_limit state. (See {ml-pull}2844[#2844].) === Bug Fixes From c5db17a712b4982a8939ca7bdb3431401f911570 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:00:19 +0200 Subject: [PATCH 12/21] remove resource monitor from parameter init object --- include/model/CBucketGatherer.h | 32 ++++++-------- include/model/CDataGatherer.h | 44 ++----------------- include/model/CEventRateBucketGatherer.h | 28 ++---------- include/model/CMetricBucketGatherer.h | 19 ++------ lib/model/CBucketGatherer.cc | 13 +++--- lib/model/CCountingModelFactory.cc | 5 +-- lib/model/CDataGatherer.cc | 21 ++------- lib/model/CEventRateModelFactory.cc | 12 +---- lib/model/CEventRatePopulationModelFactory.cc | 6 +-- lib/model/CMetricModelFactory.cc | 12 +---- lib/model/CMetricPopulationModelFactory.cc | 11 +++-- lib/model/unittest/CDetectionRuleTest.cc | 34 -------------- .../CEventRatePopulationDataGathererTest.cc | 4 +- .../CMetricPopulationDataGathererTest.cc | 10 +++-- 14 files changed, 54 insertions(+), 197 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 89383da98..10fda64c6 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -131,27 +131,25 @@ class MODEL_EXPORT CBucketGatherer { using TMetricCategoryVec = std::vector; using TTimeVec = std::vector; using TTimeVecCItr = TTimeVec::const_iterator; - using TOptionalResourceMonitorCRef = - std::optional>; struct SBucketGathererInitData { - static SBucketGathererInitData emptyData() { - return {.s_SummaryCountFieldName = "", - .s_PersonFieldName = "", - .s_AttributeFieldName = "", - .s_ValueFieldName = "", - .s_InfluenceFieldNames = {}, - .s_StartTime = 0, - .s_SampleOverrideCount = 0}; - } + // The name of the field holding the summary count. const std::string& s_SummaryCountFieldName; + // The name of the field which identifies people. const std::string& s_PersonFieldName; + // The name of the field which defines the person attributes. const std::string& s_AttributeFieldName; + // The name of the field which contains the metric values. const std::string& s_ValueFieldName; + // The field names for which we will compute influences. const TStrVec& s_InfluenceFieldNames; + // The start of the time interval for which to gather data. core_t::TTime s_StartTime; + // Override for the number of measurements + // in a statistic. (Note that this is intended for testing only.) + // A zero value means that the data gatherer class will determine + // an appropriate value for the bucket length and data rate. unsigned int s_SampleOverrideCount; - TOptionalResourceMonitorCRef s_ResourceMonitor; }; public: @@ -164,10 +162,8 @@ class MODEL_EXPORT CBucketGatherer { //! Create a new data series gatherer. //! //! \param[in] dataGatherer The owning data gatherer. - //! \param[in] startTime The start of the time interval for which - //! to gather data. - //! \param[in] numberInfluencers The number of result influencers - //! for which to gather data. + //! \param[in] bucketGathererInitData The parameter initialization object + //! for the bucket gatherer. CBucketGatherer(CDataGatherer& dataGatherer, const SBucketGathererInitData& bucketGathererInitData); @@ -243,7 +239,7 @@ class MODEL_EXPORT CBucketGatherer { CResourceMonitor& resourceMonitor) = 0; //! Record the arrival of \p data at \p time. - bool addEventData(CEventData& data); + bool addEventData(CEventData& data, const CResourceMonitor& resourceMonitor); //! Roll time forwards to \p time. void timeNow(core_t::TTime time); @@ -475,8 +471,6 @@ class MODEL_EXPORT CBucketGatherer { //! The influencing field value counts per person and/or attribute. TSizeSizePrOptionalStrPrUInt64UMapVecQueue m_InfluencerCounts; - - TOptionalResourceMonitorCRef m_ResourceMonitor; }; } } diff --git a/include/model/CDataGatherer.h b/include/model/CDataGatherer.h index 5dcb46c61..dabe633b8 100644 --- a/include/model/CDataGatherer.h +++ b/include/model/CDataGatherer.h @@ -146,51 +146,24 @@ class MODEL_EXPORT CDataGatherer { //! \param[in] summaryMode Indicates whether the data being gathered //! are already summarized by an external aggregation process. //! \param[in] modelParams The global configuration parameters. - //! \param[in] summaryCountFieldName If \p summaryMode is E_Manual - //! then this is the name of the field holding the summary count. //! \param[in] partitionFieldValue The value of the field which splits //! the data. - //! \param[in] personFieldName The name of the field which identifies - //! people. - //! \param[in] attributeFieldName The name of the field which defines - //! the person attributes. - //! \param[in] valueFieldName The name of the field which contains - //! the metric values. - //! \param[in] influenceFieldNames The field names for which we will - //! compute influences. //! \param[in] key The key of the search for which to gatherer data. //! \param[in] features The features of the data to model. - //! \param[in] startTime The start of the time interval for which - //! to gather data. - //! \param[in] sampleCountOverride for the number of measurements - //! in a statistic. (Note that this is intended for testing only.) - //! A zero value means that the data gatherer class will determine - //! an appropriate value for the bucket length and data rate. + //! \param[in] bucketGathererInitData The parameter initialization object for the bucket gatherer. CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - // const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - // const std::string& personFieldName, - // const std::string& attributeFieldName, - // const std::string& valueFieldName, - // const TStrVec& influenceFieldNames, const CSearchKey& key, const TFeatureVec& features, - // core_t::TTime startTime, - // int sampleCountOverride const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData); //! Construct from a state document. CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - // const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - // const std::string& personFieldName, - // const std::string& attributeFieldName, - // const std::string& valueFieldName, - // const TStrVec& influenceFieldNames, const CSearchKey& key, const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); @@ -332,6 +305,7 @@ class MODEL_EXPORT CDataGatherer { //! containing \p time. //! //! \param[in] time The time of interest. + //! \param[in] bucketLength The length of the bucketing interval. //! \param[out] result Filled in with the feature data at \p time. //! \tparam T The type of the feature data. template @@ -677,21 +651,11 @@ class MODEL_EXPORT CDataGatherer { private: //! Restore state from supplied traverser. - bool acceptRestoreTraverser(/*const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames,*/ - const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, + bool acceptRestoreTraverser(const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); //! Restore a bucket gatherer from the supplied traverser. - bool restoreBucketGatherer(/*const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames,*/ - const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, + bool restoreBucketGatherer(const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); //! Persist a bucket gatherer by passing information to the supplied diff --git a/include/model/CEventRateBucketGatherer.h b/include/model/CEventRateBucketGatherer.h index ca176552e..b63923050 100644 --- a/include/model/CEventRateBucketGatherer.h +++ b/include/model/CEventRateBucketGatherer.h @@ -111,35 +111,13 @@ class MODEL_EXPORT CEventRateBucketGatherer final : public CBucketGatherer { //! Create an event rate bucket gatherer. //! //! \param[in] dataGatherer The owning data gatherer. - //! \param[in] summaryCountFieldName If summaryMode is E_Manual - //! then this is the name of the field holding the summary count. - //! \param[in] personFieldName The name of the field which identifies - //! people. - //! \param[in] attributeFieldName The name of the field which defines - //! the person attributes. - //! \param[in] valueFieldName The name of the field which contains - //! the metric values. - //! \param[in] influenceFieldNames The field names for which we will - //! compute influences. - //! \param[in] startTime The start of the time interval for which - //! to gather data. + //! \param[in] bucketGathererInitData The parameter initialization object. CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData - /*const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames, - core_t::TTime startTime*/); + const SBucketGathererInitData& bucketGathererInitData); //! Construct from a state document. CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, - /*const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames,*/ + const SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); //! Create a copy that will result in the same persisted state as the diff --git a/include/model/CMetricBucketGatherer.h b/include/model/CMetricBucketGatherer.h index a577f5129..2b97c31bd 100644 --- a/include/model/CMetricBucketGatherer.h +++ b/include/model/CMetricBucketGatherer.h @@ -53,24 +53,13 @@ class MODEL_EXPORT CMetricBucketGatherer final : public CBucketGatherer { //! Create a new population metric data gatherer. //! //! \param[in] dataGatherer The owning data gatherer. - //! \param[in] summaryCountFieldName If \p summaryMode is E_Manual - //! then this is the name of the field holding the summary count. - //! \param[in] personFieldName The name of the field which identifies - //! people. - //! \param[in] attributeFieldName The name of the field which defines - //! the person attributes. - //! \param[in] valueFieldName The name of the field which contains - //! the metric values. - //! \param[in] influenceFieldNames The field names for which we will - //! compute influences. - //! \param[in] startTime The start of the time interval for which - //! to gather data. - CMetricBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& initData); + //! \param[in] initData The parameter initialization object for the bucket + //! gatherer. + CMetricBucketGatherer(CDataGatherer& dataGatherer, const SBucketGathererInitData& initData); //! Construct from a state document. CMetricBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& initData, + const SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser); //! Create a copy that will result in the same persisted state as the diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index 40a396f79..d71cfc1e6 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -9,7 +9,6 @@ * limitation. */ -#include "model/CResourceMonitor.h" #include #include @@ -24,6 +23,7 @@ #include #include +#include #include @@ -218,8 +218,7 @@ CBucketGatherer::CBucketGatherer(CDataGatherer& dataGatherer, dataGatherer.params().s_BucketLength, initData.s_StartTime, TSizeSizePrOptionalStrPrUInt64UMapVec( - initData.s_InfluenceFieldNames.size())), - m_ResourceMonitor(initData.s_ResourceMonitor) { + initData.s_InfluenceFieldNames.size())) { } CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& other) @@ -227,14 +226,13 @@ CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& o m_EarliestTime(other.m_EarliestTime), m_BucketStart(other.m_BucketStart), m_PersonAttributeCounts(other.m_PersonAttributeCounts), m_PersonAttributeExplicitNulls(other.m_PersonAttributeExplicitNulls), - m_InfluencerCounts(other.m_InfluencerCounts), - m_ResourceMonitor(other.m_ResourceMonitor) { + m_InfluencerCounts(other.m_InfluencerCounts) { if (!isForPersistence) { LOG_ABORT(<< "This constructor only creates clones for persistence"); } } -bool CBucketGatherer::addEventData(CEventData& data) { +bool CBucketGatherer::addEventData(CEventData& data, const CResourceMonitor& resourceMonitor) { core_t::TTime time = data.time(); if (time < this->earliestBucketStartTime()) { @@ -297,8 +295,7 @@ bool CBucketGatherer::addEventData(CEventData& data) { if (influence) { const std::string& inf = *influence; canonicalInfluences[i] = inf; - if (count > 0 && m_ResourceMonitor && - m_ResourceMonitor->get().areAllocationsAllowed()) { + if (count > 0 && resourceMonitor.areAllocationsAllowed()) { influencerCounts[i] .emplace(boost::unordered::piecewise_construct, boost::make_tuple(pidCid, inf), diff --git a/lib/model/CCountingModelFactory.cc b/lib/model/CCountingModelFactory.cc index e4895a213..0cdf5ce33 100644 --- a/lib/model/CCountingModelFactory.cc +++ b/lib/model/CCountingModelFactory.cc @@ -68,8 +68,7 @@ CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initD EMPTY_STRING, {}, initData.s_StartTime, - 0, - this->resourceMonitor()}; + 0}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -79,7 +78,7 @@ CDataGatherer* CCountingModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0, this->resourceMonitor()}; + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CDataGatherer.cc b/lib/model/CDataGatherer.cc index eb4a41c66..c2cb00a6e 100644 --- a/lib/model/CDataGatherer.cc +++ b/lib/model/CDataGatherer.cc @@ -170,12 +170,7 @@ CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - // const std::string& summaryCountFieldName, const std::string& partitionFieldValue, - // const std::string& personFieldName, - // const std::string& attributeFieldName, - // const std::string& valueFieldName, - // const TStrVec& influenceFieldNames, const CSearchKey& key, const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) @@ -308,7 +303,7 @@ bool CDataGatherer::addArrival(const TStrCPtrVec& fieldValues, return false; } - return m_BucketGatherer->addEventData(data); + return m_BucketGatherer->addEventData(data, resourceMonitor); } void CDataGatherer::sampleNow(core_t::TTime sampleBucketStart) { @@ -731,12 +726,7 @@ bool CDataGatherer::checkInvariants() const { return true; } -bool CDataGatherer::acceptRestoreTraverser(/*const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames,*/ - const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, +bool CDataGatherer::acceptRestoreTraverser(const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) { this->clear(); m_Features.clear(); @@ -774,12 +764,7 @@ bool CDataGatherer::acceptRestoreTraverser(/*const std::string& summaryCountFiel return true; } -bool CDataGatherer::restoreBucketGatherer(/*const std::string& summaryCountFieldName, - const std::string& personFieldName, - const std::string& attributeFieldName, - const std::string& valueFieldName, - const TStrVec& influenceFieldNames,*/ - const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, +bool CDataGatherer::restoreBucketGatherer(const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) { do { const std::string& name = traverser.name(); diff --git a/lib/model/CEventRateModelFactory.cc b/lib/model/CEventRateModelFactory.cc index 3090408c4..a3e8423b6 100644 --- a/lib/model/CEventRateModelFactory.cc +++ b/lib/model/CEventRateModelFactory.cc @@ -99,8 +99,7 @@ CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& init m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount, - this->resourceMonitor()}; + initData.s_SampleOverrideCount}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -110,14 +109,7 @@ CDataGatherer* CEventRateModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, - m_PersonFieldName, - EMPTY_STRING, - m_ValueFieldName, - m_InfluenceFieldNames, - 0, - 0, - this->resourceMonitor()}; + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CEventRatePopulationModelFactory.cc b/lib/model/CEventRatePopulationModelFactory.cc index 89de455c2..ba3f25c1a 100644 --- a/lib/model/CEventRatePopulationModelFactory.cc +++ b/lib/model/CEventRatePopulationModelFactory.cc @@ -100,8 +100,7 @@ CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitialization .s_ValueFieldName = m_ValueFieldName, .s_InfluenceFieldNames = m_InfluenceFieldNames, .s_StartTime = initData.s_StartTime, - .s_SampleOverrideCount = 0, - this->resourceMonitor()}; + .s_SampleOverrideCount = 0}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -117,8 +116,7 @@ CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionF .s_ValueFieldName = m_ValueFieldName, .s_InfluenceFieldNames = m_InfluenceFieldNames, .s_StartTime = 0, - .s_SampleOverrideCount = 0, - this->resourceMonitor()}; + .s_SampleOverrideCount = 0}; return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CMetricModelFactory.cc b/lib/model/CMetricModelFactory.cc index 3357925f0..7c6b5fcc2 100644 --- a/lib/model/CMetricModelFactory.cc +++ b/lib/model/CMetricModelFactory.cc @@ -99,8 +99,7 @@ CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initDat m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount, - this->resourceMonitor()}; + initData.s_SampleOverrideCount}; return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -110,14 +109,7 @@ CDataGatherer* CMetricModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, - m_PersonFieldName, - EMPTY_STRING, - m_ValueFieldName, - m_InfluenceFieldNames, - 0, - 0, - this->resourceMonitor()}; + m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/CMetricPopulationModelFactory.cc b/lib/model/CMetricPopulationModelFactory.cc index cced96384..30d8df416 100644 --- a/lib/model/CMetricPopulationModelFactory.cc +++ b/lib/model/CMetricPopulationModelFactory.cc @@ -93,10 +93,10 @@ CMetricPopulationModelFactory::makeModel(const SModelInitializationData& initDat CDataGatherer* CMetricPopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ - m_SummaryCountFieldName, m_PersonFieldName, - m_AttributeFieldName, m_ValueFieldName, - m_InfluenceFieldNames, initData.s_StartTime, - initData.s_SampleOverrideCount, this->resourceMonitor()}; + m_SummaryCountFieldName, m_PersonFieldName, + m_AttributeFieldName, m_ValueFieldName, + m_InfluenceFieldNames, initData.s_StartTime, + initData.s_SampleOverrideCount}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); @@ -112,8 +112,7 @@ CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFiel m_ValueFieldName, m_InfluenceFieldNames, 0, - 0, - this->resourceMonitor()}; + 0}; return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); diff --git a/lib/model/unittest/CDetectionRuleTest.cc b/lib/model/unittest/CDetectionRuleTest.cc index 8821d2573..0fdd8af73 100644 --- a/lib/model/unittest/CDetectionRuleTest.cc +++ b/lib/model/unittest/CDetectionRuleTest.cc @@ -794,40 +794,6 @@ BOOST_FIXTURE_TEST_CASE(testRuleActions, CTestFixture) { 0, 0, 100)); } -// TMockModelPtr initializeModel(ml::model::CResourceMonitor& resourceMonitor) { -// core_t::TTime bucketLength{600}; -// model::SModelParams params{bucketLength}; -// model::CSearchKey key; -// model_t::TFeatureVec features; -// // Initialize mock model -// model::CAnomalyDetectorModel::TDataGathererPtr gatherer; -// -// features.assign(1, model_t::E_IndividualSumByBucketAndPerson); -// -// gatherer = CDataGathererBuilder(model_t::analysisCategory(features[0]), features, params, key, 0) -// .personFieldName("p") -// .buildSharedPtr(); -// std::string person("p1"); -// bool addedPerson{false}; -// gatherer->addPerson(person, resourceMonitor, addedPerson); -// -// TMockModelPtr model{new model::CMockModel( -// params, gatherer, {/* we don't care about influence */})}; -// -// maths::time_series::CTimeSeriesDecomposition trend; -// maths::common::CNormalMeanPrecConjugate prior{ -// maths::common::CNormalMeanPrecConjugate::nonInformativePrior(maths_t::E_ContinuousData)}; -// maths::common::CModelParams timeSeriesModelParams{ -// bucketLength, 1.0, 0.001, 0.2, 6 * core::constants::HOUR, 24 * core::constants::HOUR}; -// std::unique_ptr timeSeriesModel = -// std::make_unique( -// timeSeriesModelParams, 0, trend, prior); -// model::CMockModel::TMathsModelUPtrVec models; -// models.emplace_back(std::move(timeSeriesModel)); -// model->mockTimeSeriesModels(std::move(models)); -// return model; -// } - BOOST_FIXTURE_TEST_CASE(testRuleTimeShiftShouldShiftTimeSeriesModelState, CTestFixture) { test::CRandomNumbers rng; diff --git a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc index c3f0fc483..f10095646 100644 --- a/lib/model/unittest/CEventRatePopulationDataGathererTest.cc +++ b/lib/model/unittest/CEventRatePopulationDataGathererTest.cc @@ -783,7 +783,9 @@ void testPersistDataGatherer(const CDataGatherer& origDataGatherer, std::istringstream origJsonStrm("{\"topLevel\" : " + origJson.str() + "}"); core::CJsonStateRestoreTraverser traverser(origJsonStrm); - auto bucketGathererInitData = CBucketGatherer::SBucketGathererInitData::emptyData(); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; + CDataGatherer restoredDataGatherer(model_t::E_PopulationEventRate, model_t::E_None, params, EMPTY_STRING, searchKey, bucketGathererInitData, traverser); diff --git a/lib/model/unittest/CMetricPopulationDataGathererTest.cc b/lib/model/unittest/CMetricPopulationDataGathererTest.cc index 738c49f5d..2fe997570 100644 --- a/lib/model/unittest/CMetricPopulationDataGathererTest.cc +++ b/lib/model/unittest/CMetricPopulationDataGathererTest.cc @@ -1029,10 +1029,12 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { // The traverser expects the state json in a embedded document std::stringstream origJsonStrm{"{\"topLevel\" : " + origJson.str() + "}"}; core::CJsonStateRestoreTraverser traverser(origJsonStrm); - auto bucketInitData = CBucketGatherer::SBucketGathererInitData::emptyData(); - CDataGatherer restoredDataGatherer(model_t::E_PopulationMetric, - model_t::E_None, params, EMPTY_STRING, - searchKey, bucketInitData, traverser); + CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ + EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; + + CDataGatherer restoredDataGatherer(model_t::E_PopulationMetric, model_t::E_None, + params, EMPTY_STRING, searchKey, + bucketGathererInitData, traverser); // The JSON representation of the new data gatherer should be the same as the // original From d23606083b9b08c1f1bb704d718756b4e4743794 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:07:50 +0200 Subject: [PATCH 13/21] remove resource monitor from addEventData --- docs/CHANGELOG.asciidoc | 1 - include/model/CBucketGatherer.h | 2 +- include/model/CModelFactory.h | 7 ------- lib/api/CAnomalyJob.cc | 7 ++----- lib/model/CBucketGatherer.cc | 5 ++--- lib/model/CDataGatherer.cc | 2 +- 6 files changed, 6 insertions(+), 18 deletions(-) diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index cc0ec094f..75117ca06 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -33,7 +33,6 @@ === Enhancements * Track memory used in the hierarchical results normalizer. (See {ml-pull}2831[#2831].) -* Improve adherence to memory limits in the hard_limit state. (See {ml-pull}2844[#2844].) === Bug Fixes diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 10fda64c6..5de673067 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -239,7 +239,7 @@ class MODEL_EXPORT CBucketGatherer { CResourceMonitor& resourceMonitor) = 0; //! Record the arrival of \p data at \p time. - bool addEventData(CEventData& data, const CResourceMonitor& resourceMonitor); + bool addEventData(CEventData& data); //! Roll time forwards to \p time. void timeNow(core_t::TTime time); diff --git a/include/model/CModelFactory.h b/include/model/CModelFactory.h index 264fd8a09..2d9553fd2 100644 --- a/include/model/CModelFactory.h +++ b/include/model/CModelFactory.h @@ -25,7 +25,6 @@ #include #include #include -#include #include namespace ml { @@ -376,10 +375,6 @@ class MODEL_EXPORT CModelFactory { //! Get the minimum seasonal variance scale, specific to the model virtual double minimumSeasonalVarianceScale() const = 0; - void resourceMonitor(const TResourceMonitorCRef& resourceMonitor) const; - - TOptionalResourceMonitorCRef resourceMonitor() const; - protected: using TMultivariatePriorUPtrVec = std::vector; using TOptionalSearchKey = std::optional; @@ -462,8 +457,6 @@ class MODEL_EXPORT CModelFactory { //! A cache of influence calculators for collections of features. mutable TStrFeatureVecPrInfluenceCalculatorCPtrMap m_InfluenceCalculatorCache; - - mutable TOptionalResourceMonitorCRef m_ResourceMonitor; }; } } diff --git a/lib/api/CAnomalyJob.cc b/lib/api/CAnomalyJob.cc index 1a16a1ec9..3a8c06be2 100644 --- a/lib/api/CAnomalyJob.cc +++ b/lib/api/CAnomalyJob.cc @@ -1610,11 +1610,8 @@ CAnomalyJob::detectorForKey(bool isRestoring, << partition << '\'' << ", time " << time); LOG_TRACE(<< "Detector count " << m_Detectors.size()); - auto factory = m_ModelConfig.factory(key); - factory->resourceMonitor(resourceMonitor); - - detector = ml::api::CAnomalyJob::makeDetector(m_ModelConfig, m_Limits, - partition, time, factory); + detector = ml::api::CAnomalyJob::makeDetector( + m_ModelConfig, m_Limits, partition, time, m_ModelConfig.factory(key)); if (detector == nullptr) { // This should never happen as CAnomalyDetectorUtils::makeDetector() // contracts to never return NULL diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index d71cfc1e6..8dabad278 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -23,7 +23,6 @@ #include #include -#include #include @@ -232,7 +231,7 @@ CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& o } } -bool CBucketGatherer::addEventData(CEventData& data, const CResourceMonitor& resourceMonitor) { +bool CBucketGatherer::addEventData(CEventData& data) { core_t::TTime time = data.time(); if (time < this->earliestBucketStartTime()) { @@ -295,7 +294,7 @@ bool CBucketGatherer::addEventData(CEventData& data, const CResourceMonitor& res if (influence) { const std::string& inf = *influence; canonicalInfluences[i] = inf; - if (count > 0 && resourceMonitor.areAllocationsAllowed()) { + if (count > 0) { influencerCounts[i] .emplace(boost::unordered::piecewise_construct, boost::make_tuple(pidCid, inf), diff --git a/lib/model/CDataGatherer.cc b/lib/model/CDataGatherer.cc index c2cb00a6e..1b3c80591 100644 --- a/lib/model/CDataGatherer.cc +++ b/lib/model/CDataGatherer.cc @@ -303,7 +303,7 @@ bool CDataGatherer::addArrival(const TStrCPtrVec& fieldValues, return false; } - return m_BucketGatherer->addEventData(data, resourceMonitor); + return m_BucketGatherer->addEventData(data); } void CDataGatherer::sampleNow(core_t::TTime sampleBucketStart) { From 0e874209981fe89192d8295a527243bcc6a42db6 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:16:41 +0200 Subject: [PATCH 14/21] minor formatting issues --- include/model/CModelFactory.h | 3 --- lib/model/unittest/CMetricDataGathererTest.cc | 3 --- lib/model/unittest/CMetricPopulationDataGathererTest.cc | 1 - 3 files changed, 7 deletions(-) diff --git a/include/model/CModelFactory.h b/include/model/CModelFactory.h index 2d9553fd2..70792cdec 100644 --- a/include/model/CModelFactory.h +++ b/include/model/CModelFactory.h @@ -17,7 +17,6 @@ #include #include -#include #include #include #include @@ -104,8 +103,6 @@ class MODEL_EXPORT CModelFactory { using TStrDetectionRulePr = std::pair; using TStrDetectionRulePrVec = std::vector; using TStrDetectionRulePrVecCRef = std::reference_wrapper; - using TResourceMonitorCRef = std::reference_wrapper; - using TOptionalResourceMonitorCRef = std::optional; public: //! Wrapper around the model initialization data. diff --git a/lib/model/unittest/CMetricDataGathererTest.cc b/lib/model/unittest/CMetricDataGathererTest.cc index fc9e4472a..76feaf054 100644 --- a/lib/model/unittest/CMetricDataGathererTest.cc +++ b/lib/model/unittest/CMetricDataGathererTest.cc @@ -689,7 +689,6 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { gatherer.recyclePeople(peopleToRemove); CDataGatherer expectedGatherer = - CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) .build(); BOOST_REQUIRE_EQUAL(0, addPerson("p3", expectedGatherer, m_ResourceMonitor)); @@ -722,7 +721,6 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { gatherer.recyclePeople(peopleToRemove); CDataGatherer expectedGatherer = - CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) .build(); @@ -753,7 +751,6 @@ BOOST_FIXTURE_TEST_CASE(testRemovePeople, CTestFixture) { gatherer.recyclePeople(peopleToRemove); CDataGatherer const expectedGatherer = - CDataGathererBuilder(model_t::E_Metric, features, params, KEY, startTime) .build(); diff --git a/lib/model/unittest/CMetricPopulationDataGathererTest.cc b/lib/model/unittest/CMetricPopulationDataGathererTest.cc index 2fe997570..5dbc6d734 100644 --- a/lib/model/unittest/CMetricPopulationDataGathererTest.cc +++ b/lib/model/unittest/CMetricPopulationDataGathererTest.cc @@ -998,7 +998,6 @@ BOOST_FIXTURE_TEST_CASE(testPersistence, CTestFixture) { features.push_back(model_t::E_PopulationMaxByPersonAndAttribute); features.push_back(model_t::E_PopulationHighSumByBucketPersonAndAttribute); CDataGatherer origDataGatherer = - CDataGathererBuilder(model_t::E_PopulationMetric, features, params, searchKey, startTime) .build(); From eceb0d91a1ab328a0e9b5c5d3e87eb56e80e0f0a Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:21:24 +0200 Subject: [PATCH 15/21] fix build failure --- lib/model/CModelFactory.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/model/CModelFactory.cc b/lib/model/CModelFactory.cc index 81184ed22..70737d556 100644 --- a/lib/model/CModelFactory.cc +++ b/lib/model/CModelFactory.cc @@ -291,13 +291,6 @@ void CModelFactory::annotationsEnabled(bool enabled) { m_ModelParams.s_AnnotationsEnabled = enabled; } -void CModelFactory::resourceMonitor(const TResourceMonitorCRef& resourceMonitor) const { - m_ResourceMonitor = resourceMonitor; -} -CModelFactory::TOptionalResourceMonitorCRef CModelFactory::resourceMonitor() const { - return m_ResourceMonitor; -} - CModelFactory::TInterimBucketCorrectorPtr CModelFactory::interimBucketCorrector() const { TInterimBucketCorrectorPtr result{m_InterimBucketCorrector.lock()}; if (result == nullptr) { From 09fe359afdbcfab672e01d87d557596f1a6ddccd Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:49:31 +0200 Subject: [PATCH 16/21] sonar qube linting --- include/model/CBucketGatherer.h | 3 +-- include/model/CEventRateBucketGatherer.h | 6 +++--- include/model/CMetricBucketGatherer.h | 4 ++-- lib/model/CDataGatherer.cc | 7 ++++--- lib/model/CEventRateBucketGatherer.cc | 10 ++++------ lib/model/CMetricBucketGatherer.cc | 9 ++++----- lib/model/unittest/ModelTestHelpers.h | 6 +----- 7 files changed, 19 insertions(+), 26 deletions(-) diff --git a/include/model/CBucketGatherer.h b/include/model/CBucketGatherer.h index 5de673067..82000ea0d 100644 --- a/include/model/CBucketGatherer.h +++ b/include/model/CBucketGatherer.h @@ -32,7 +32,6 @@ #include #include #include -#include #include namespace ml { @@ -98,7 +97,7 @@ class MODEL_EXPORT CBucketGatherer { //! \brief Hashes a ((size_t, size_t), string*) pair. struct MODEL_EXPORT SSizeSizePrOptionalStrPrHash { std::size_t operator()(const TSizeSizePrOptionalStrPr& key) const { - std::uint64_t seed = core::CHashing::hashCombine( + std::uint64_t const seed = core::CHashing::hashCombine( static_cast(key.first.first), static_cast(key.first.second)); return core::CHashing::hashCombine(seed, s_Hasher(key.second)); diff --git a/include/model/CEventRateBucketGatherer.h b/include/model/CEventRateBucketGatherer.h index b63923050..947c74fef 100644 --- a/include/model/CEventRateBucketGatherer.h +++ b/include/model/CEventRateBucketGatherer.h @@ -457,13 +457,13 @@ class MODEL_EXPORT CEventRateBucketGatherer final : public CBucketGatherer { TStrVec m_FieldNames; //! The position of the first influencer field - std::size_t m_BeginInfluencingFields; + std::size_t m_BeginInfluencingFields{0}; //! The position of the first count/value field. - std::size_t m_BeginValueField; + std::size_t m_BeginValueField{0}; //! The position of the field holding the summarised count. - std::size_t m_BeginSummaryFields; + std::size_t m_BeginSummaryFields{0}; //! The data features we are gathering. TCategoryAnyMap m_FeatureData; diff --git a/include/model/CMetricBucketGatherer.h b/include/model/CMetricBucketGatherer.h index 2b97c31bd..bd309d8aa 100644 --- a/include/model/CMetricBucketGatherer.h +++ b/include/model/CMetricBucketGatherer.h @@ -284,10 +284,10 @@ class MODEL_EXPORT CMetricBucketGatherer final : public CBucketGatherer { TStrVec m_FieldNames; //! The position of the first influencing field. - std::size_t m_BeginInfluencingFields; + std::size_t m_BeginInfluencingFields{0}; //! The position of the first count/value field. - std::size_t m_BeginValueFields; + std::size_t m_BeginValueFields{0}; //! For summarized values, this stores the metric categories //! corresponding to the summarized field names in m_FieldNames; diff --git a/lib/model/CDataGatherer.cc b/lib/model/CDataGatherer.cc index 1b3c80591..f52e3640c 100644 --- a/lib/model/CDataGatherer.cc +++ b/lib/model/CDataGatherer.cc @@ -185,9 +185,10 @@ CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, counter_t::E_TSADNumberNewAttributesNotAllowed, counter_t::E_TSADNumberNewAttributesRecycled), m_Population(detail::isPopulation(gathererType)), m_UseNull(key.useNull()) { - if (traverser.traverseSubLevel(std::bind(&CDataGatherer::acceptRestoreTraverser, - this, std::cref(bucketGathererInitData), - std::placeholders::_1)) == false) { + auto func = [this, &bucketGathererInitData](core::CStateRestoreTraverser& traverser_) { + return acceptRestoreTraverser(bucketGathererInitData, traverser_); + }; + if (traverser.traverseSubLevel(func) == false) { LOG_ERROR(<< "Failed to correctly restore data gatherer"); } } diff --git a/lib/model/CEventRateBucketGatherer.cc b/lib/model/CEventRateBucketGatherer.cc index aa3ab46ab..a01ddc9cd 100644 --- a/lib/model/CEventRateBucketGatherer.cc +++ b/lib/model/CEventRateBucketGatherer.cc @@ -727,18 +727,16 @@ void registerMemoryCallbacks() { } // unnamed:: CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& initData) - : CBucketGatherer(dataGatherer, initData), m_BeginInfluencingFields(0), - m_BeginValueField(0), m_BeginSummaryFields(0) { + const SBucketGathererInitData& initData) + : CBucketGatherer(dataGatherer, initData) { this->initializeFieldNames(initData); this->initializeFeatureData(); } CEventRateBucketGatherer::CEventRateBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& initData, + const SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) - : CBucketGatherer(dataGatherer, initData), m_BeginInfluencingFields(0), - m_BeginValueField(0), m_BeginSummaryFields(0) { + : CBucketGatherer(dataGatherer, initData) { this->initializeFieldNames(initData); if (traverser.traverseSubLevel(std::bind(&CEventRateBucketGatherer::acceptRestoreTraverser, this, std::placeholders::_1)) == false) { diff --git a/lib/model/CMetricBucketGatherer.cc b/lib/model/CMetricBucketGatherer.cc index 830787364..19cfb8ccb 100644 --- a/lib/model/CMetricBucketGatherer.cc +++ b/lib/model/CMetricBucketGatherer.cc @@ -925,20 +925,19 @@ struct SReleaseMemory { } // unnamed:: CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& initData) + const SBucketGathererInitData& initData) : CBucketGatherer(dataGatherer, initData), - m_ValueFieldName(initData.s_ValueFieldName), m_BeginInfluencingFields(0), - m_BeginValueFields(0) { + m_ValueFieldName(initData.s_ValueFieldName) { this->initializeFieldNamesPart1(initData); this->initializeFieldNamesPart2(initData); this->initializeFeatureData(); } CMetricBucketGatherer::CMetricBucketGatherer(CDataGatherer& dataGatherer, - const CBucketGatherer::SBucketGathererInitData& initData, + const SBucketGathererInitData& initData, core::CStateRestoreTraverser& traverser) : CBucketGatherer(dataGatherer, initData), - m_ValueFieldName(initData.s_ValueFieldName), m_BeginValueFields(0) { + m_ValueFieldName(initData.s_ValueFieldName) { this->initializeFieldNamesPart1(initData); if (traverser.traverseSubLevel(std::bind(&CMetricBucketGatherer::acceptRestoreTraverser, this, std::placeholders::_1)) == false) { diff --git a/lib/model/unittest/ModelTestHelpers.h b/lib/model/unittest/ModelTestHelpers.h index 3898cb094..69554cac3 100644 --- a/lib/model/unittest/ModelTestHelpers.h +++ b/lib/model/unittest/ModelTestHelpers.h @@ -110,10 +110,6 @@ class CDataGathererBuilder { m_PartitionFieldValue, m_SearchKey, m_Features, bucketGathererInitData}; } - CDataGathererBuilder& partitionFieldValue(std::string_view partitionFieldValue) { - m_PartitionFieldValue = partitionFieldValue; - return *this; - } std::shared_ptr buildSharedPtr() const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ @@ -129,7 +125,7 @@ class CDataGathererBuilder { m_Features, bucketGathererInitData); } - CDataGathererBuilder& partitionFieldValue(const std::string& partitionFieldValue) { + CDataGathererBuilder& partitionFieldValue(std::string_view partitionFieldValue) { m_PartitionFieldValue = partitionFieldValue; return *this; } From 5b6a0d9b39aa02ff7bb3e034f2eb2a8dc9b7c841 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 15:59:48 +0200 Subject: [PATCH 17/21] change data gatherer factory --- include/model/CCountingModelFactory.h | 6 +++--- include/model/CEventRateModelFactory.h | 6 +++--- include/model/CEventRatePopulationModelFactory.h | 6 +++--- include/model/CMetricModelFactory.h | 6 +++--- include/model/CMetricPopulationModelFactory.h | 6 +++--- include/model/CModelFactory.h | 7 ++++--- lib/model/CAnomalyDetector.cc | 3 +-- lib/model/CCountingModelFactory.cc | 8 ++++---- lib/model/CEventRateModelFactory.cc | 8 ++++---- lib/model/CEventRatePopulationModelFactory.cc | 8 ++++---- lib/model/CMetricModelFactory.cc | 8 ++++---- lib/model/CMetricPopulationModelFactory.cc | 8 ++++---- lib/model/unittest/CModelTestFixtureBase.h | 2 +- lib/model/unittest/CResourceLimitTest.cc | 4 ++-- 14 files changed, 43 insertions(+), 43 deletions(-) diff --git a/include/model/CCountingModelFactory.h b/include/model/CCountingModelFactory.h index bd2ccb3da..372f970a8 100644 --- a/include/model/CCountingModelFactory.h +++ b/include/model/CCountingModelFactory.h @@ -68,15 +68,15 @@ class MODEL_EXPORT CCountingModelFactory : public CModelFactory { //! \param[in] initData The parameters needed to initialize the data //! gatherer. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const SGathererInitializationData& initData) const override; + TDataGathererPtr makeDataGatherer(const SGathererInitializationData& initData) const override; //! Make a new event rate data gatherer from part of a state document. //! //! \param[in] partitionFieldValue The partition field value. //! \param[in,out] traverser A state document traverser. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const std::string& partitionFieldValue, - core::CStateRestoreTraverser& traverser) const override; + TDataGathererPtr makeDataGatherer(const std::string& partitionFieldValue, + core::CStateRestoreTraverser& traverser) const override; //@} //! \name Defaults diff --git a/include/model/CEventRateModelFactory.h b/include/model/CEventRateModelFactory.h index 0d7981ced..f94dc8cc0 100644 --- a/include/model/CEventRateModelFactory.h +++ b/include/model/CEventRateModelFactory.h @@ -69,15 +69,15 @@ class MODEL_EXPORT CEventRateModelFactory final : public CModelFactory { //! \param[in] initData The parameters needed to initialize the data //! gatherer. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const SGathererInitializationData& initData) const override; + TDataGathererPtr makeDataGatherer(const SGathererInitializationData& initData) const override; //! Make a new event rate data gatherer from part of a state document. //! //! \param[in] partitionFieldValue The partition field value. //! \param[in,out] traverser A state document traverser. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const std::string& partitionFieldValue, - core::CStateRestoreTraverser& traverser) const override; + TDataGathererPtr makeDataGatherer(const std::string& partitionFieldValue, + core::CStateRestoreTraverser& traverser) const override; //@} //! \name Defaults diff --git a/include/model/CEventRatePopulationModelFactory.h b/include/model/CEventRatePopulationModelFactory.h index 2595e3b72..974abdf4f 100644 --- a/include/model/CEventRatePopulationModelFactory.h +++ b/include/model/CEventRatePopulationModelFactory.h @@ -69,7 +69,7 @@ class MODEL_EXPORT CEventRatePopulationModelFactory final : public CModelFactory //! \param[in] initData The parameters needed to initialize the //! data gatherer. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const SGathererInitializationData& initData) const override; + TDataGathererPtr makeDataGatherer(const SGathererInitializationData& initData) const override; //! Make a new population event rate data gatherer from part of //! a state document. @@ -77,8 +77,8 @@ class MODEL_EXPORT CEventRatePopulationModelFactory final : public CModelFactory //! \param[in] partitionFieldValue The partition field value. //! \param[in,out] traverser A state document traverser. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const std::string& partitionFieldValue, - core::CStateRestoreTraverser& traverser) const override; + TDataGathererPtr makeDataGatherer(const std::string& partitionFieldValue, + core::CStateRestoreTraverser& traverser) const override; //@} //! \name Defaults diff --git a/include/model/CMetricModelFactory.h b/include/model/CMetricModelFactory.h index 52ecc127c..c14f7aca6 100644 --- a/include/model/CMetricModelFactory.h +++ b/include/model/CMetricModelFactory.h @@ -69,15 +69,15 @@ class MODEL_EXPORT CMetricModelFactory final : public CModelFactory { //! \param[in] initData The parameters needed to initialize the //! data gatherer. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const SGathererInitializationData& initData) const override; + TDataGathererPtr makeDataGatherer(const SGathererInitializationData& initData) const override; //! Make a new metric data gatherer from part of a state document. //! //! \param[in] partitionFieldValue The partition field value. //! \param[in,out] traverser A state document traverser. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const std::string& partitionFieldValue, - core::CStateRestoreTraverser& traverser) const override; + TDataGathererPtr makeDataGatherer(const std::string& partitionFieldValue, + core::CStateRestoreTraverser& traverser) const override; //@} //! \name Defaults diff --git a/include/model/CMetricPopulationModelFactory.h b/include/model/CMetricPopulationModelFactory.h index 2f8c4dbb6..caa4a5949 100644 --- a/include/model/CMetricPopulationModelFactory.h +++ b/include/model/CMetricPopulationModelFactory.h @@ -69,7 +69,7 @@ class MODEL_EXPORT CMetricPopulationModelFactory final : public CModelFactory { //! \param[in] initData The parameters needed to initialize the data //! gatherer. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const SGathererInitializationData& initData) const override; + TDataGathererPtr makeDataGatherer(const SGathererInitializationData& initData) const override; //! Make a new metric population data gatherer from part of a state //! document. @@ -77,8 +77,8 @@ class MODEL_EXPORT CMetricPopulationModelFactory final : public CModelFactory { //! \param[in] partitionFieldValue The partition field value. //! \param[in,out] traverser A state document traverser. //! \warning It is owned by the calling code. - CDataGatherer* makeDataGatherer(const std::string& partitionFieldValue, - core::CStateRestoreTraverser& traverser) const override; + TDataGathererPtr makeDataGatherer(const std::string& partitionFieldValue, + core::CStateRestoreTraverser& traverser) const override; //@} //! \name Defaults diff --git a/include/model/CModelFactory.h b/include/model/CModelFactory.h index 70792cdec..166d81674 100644 --- a/include/model/CModelFactory.h +++ b/include/model/CModelFactory.h @@ -174,7 +174,7 @@ class MODEL_EXPORT CModelFactory { //! \param[in] initData The parameters needed to initialize the //! data gatherer. //! \warning It is owned by the calling code. - virtual CDataGatherer* + virtual TDataGathererPtr makeDataGatherer(const SGathererInitializationData& initData) const = 0; //! Make a new data gatherer from part of a state document. @@ -182,8 +182,9 @@ class MODEL_EXPORT CModelFactory { //! \param[in,out] traverser A state document traverser. //! \param[in] partitionFieldValue The partition field value. //! \warning It is owned by the calling code. - virtual CDataGatherer* makeDataGatherer(const std::string& partitionFieldValue, - core::CStateRestoreTraverser& traverser) const = 0; + virtual TDataGathererPtr + makeDataGatherer(const std::string& partitionFieldValue, + core::CStateRestoreTraverser& traverser) const = 0; //@} //! \name Defaults diff --git a/lib/model/CAnomalyDetector.cc b/lib/model/CAnomalyDetector.cc index f8c36d005..2049e49c8 100644 --- a/lib/model/CAnomalyDetector.cc +++ b/lib/model/CAnomalyDetector.cc @@ -206,8 +206,7 @@ bool CAnomalyDetector::legacyModelEnsembleAcceptRestoreTraverser(const std::stri do { const std::string& name = traverser.name(); if (name == DATA_GATHERER_TAG) { - m_DataGatherer.reset( - m_ModelFactory->makeDataGatherer(partitionFieldValue, traverser)); + m_DataGatherer = m_ModelFactory->makeDataGatherer(partitionFieldValue, traverser); if (m_DataGatherer == nullptr || m_DataGatherer->checkInvariants() == false) { LOG_ERROR(<< "Failed to restore the data gatherer from " << traverser.value()); diff --git a/lib/model/CCountingModelFactory.cc b/lib/model/CCountingModelFactory.cc index 0cdf5ce33..3f4c652da 100644 --- a/lib/model/CCountingModelFactory.cc +++ b/lib/model/CCountingModelFactory.cc @@ -59,7 +59,7 @@ CCountingModelFactory::makeModel(const SModelInitializationData& initData, this->interimBucketCorrector(), traverser); } -CDataGatherer* +CModelFactory::TDataGathererPtr CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { const CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, @@ -69,17 +69,17 @@ CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initD {}, initData.s_StartTime, 0}; - return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, + return std::make_shared(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); } -CDataGatherer* +CModelFactory::TDataGathererPtr CCountingModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; - return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, + return std::make_shared(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } diff --git a/lib/model/CEventRateModelFactory.cc b/lib/model/CEventRateModelFactory.cc index a3e8423b6..579ccd998 100644 --- a/lib/model/CEventRateModelFactory.cc +++ b/lib/model/CEventRateModelFactory.cc @@ -90,7 +90,7 @@ CEventRateModelFactory::makeModel(const SModelInitializationData& initData, influenceCalculators, this->interimBucketCorrector(), traverser); } -CDataGatherer* +CModelFactory::TDataGathererPtr CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, @@ -100,17 +100,17 @@ CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& init m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount}; - return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, + return std::make_shared(model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); } -CDataGatherer* +CModelFactory::TDataGathererPtr CEventRateModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; - return new CDataGatherer(model_t::E_EventRate, m_SummaryMode, + return std::make_shared(model_t::E_EventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } diff --git a/lib/model/CEventRatePopulationModelFactory.cc b/lib/model/CEventRatePopulationModelFactory.cc index ba3f25c1a..cbda1921b 100644 --- a/lib/model/CEventRatePopulationModelFactory.cc +++ b/lib/model/CEventRatePopulationModelFactory.cc @@ -91,7 +91,7 @@ CEventRatePopulationModelFactory::makeModel(const SModelInitializationData& init influenceCalculators, this->interimBucketCorrector(), traverser); } -CDataGatherer* +CModelFactory::TDataGathererPtr CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData const bucketGathererInitData{ .s_SummaryCountFieldName = m_SummaryCountFieldName, @@ -101,12 +101,12 @@ CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitialization .s_InfluenceFieldNames = m_InfluenceFieldNames, .s_StartTime = initData.s_StartTime, .s_SampleOverrideCount = 0}; - return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, + return std::make_shared(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); } -CDataGatherer* +CModelFactory::TDataGathererPtr CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData const bucketGathererInitData{ @@ -117,7 +117,7 @@ CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionF .s_InfluenceFieldNames = m_InfluenceFieldNames, .s_StartTime = 0, .s_SampleOverrideCount = 0}; - return new CDataGatherer(model_t::E_PopulationEventRate, m_SummaryMode, + return std::make_shared(model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } diff --git a/lib/model/CMetricModelFactory.cc b/lib/model/CMetricModelFactory.cc index 7c6b5fcc2..4559dffaf 100644 --- a/lib/model/CMetricModelFactory.cc +++ b/lib/model/CMetricModelFactory.cc @@ -90,7 +90,7 @@ CMetricModelFactory::makeModel(const SModelInitializationData& initData, influenceCalculators, this->interimBucketCorrector(), traverser); } -CDataGatherer* +CModelFactory::TDataGathererPtr CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, @@ -100,17 +100,17 @@ CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initDat m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount}; - return new CDataGatherer(model_t::E_Metric, m_SummaryMode, this->modelParams(), + return std::make_shared(model_t::E_Metric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); } -CDataGatherer* +CModelFactory::TDataGathererPtr CMetricModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; - return new CDataGatherer(model_t::E_Metric, m_SummaryMode, + return std::make_shared(model_t::E_Metric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } diff --git a/lib/model/CMetricPopulationModelFactory.cc b/lib/model/CMetricPopulationModelFactory.cc index 30d8df416..b601284cf 100644 --- a/lib/model/CMetricPopulationModelFactory.cc +++ b/lib/model/CMetricPopulationModelFactory.cc @@ -90,19 +90,19 @@ CMetricPopulationModelFactory::makeModel(const SModelInitializationData& initDat influenceCalculators, this->interimBucketCorrector(), traverser); } -CDataGatherer* +CModelFactory::TDataGathererPtr CMetricPopulationModelFactory::makeDataGatherer(const SGathererInitializationData& initData) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, m_AttributeFieldName, m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount}; - return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, + return std::make_shared(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, this->searchKey(), m_Features, bucketGathererInitData); } -CDataGatherer* +CModelFactory::TDataGathererPtr CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ @@ -113,7 +113,7 @@ CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFiel m_InfluenceFieldNames, 0, 0}; - return new CDataGatherer(model_t::E_PopulationMetric, m_SummaryMode, + return std::make_shared(model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } diff --git a/lib/model/unittest/CModelTestFixtureBase.h b/lib/model/unittest/CModelTestFixtureBase.h index c78a96b36..da8881dbb 100644 --- a/lib/model/unittest/CModelTestFixtureBase.h +++ b/lib/model/unittest/CModelTestFixtureBase.h @@ -243,7 +243,7 @@ class CModelTestFixtureBase { if (sampleCount) { initData.s_SampleOverrideCount = *sampleCount; } - gatherer.reset(m_Factory->makeDataGatherer(initData)); + gatherer = m_Factory->makeDataGatherer(initData); model.reset(m_Factory->makeModel({gatherer})); BOOST_TEST_REQUIRE(model); diff --git a/lib/model/unittest/CResourceLimitTest.cc b/lib/model/unittest/CResourceLimitTest.cc index 6d9253d49..853325399 100644 --- a/lib/model/unittest/CResourceLimitTest.cc +++ b/lib/model/unittest/CResourceLimitTest.cc @@ -292,7 +292,7 @@ TAddPersonDataFunc createModel(model_t::EModelType modelType, features.push_back(model_t::E_IndividualCountByBucketAndPerson); factory->features(features); - gatherer.reset(factory->makeDataGatherer(firstTime)); + gatherer = factory->makeDataGatherer(firstTime); const maths::common::CMultinomialConjugate conjugate; std::shared_ptr model_ = std::make_shared( @@ -323,7 +323,7 @@ TAddPersonDataFunc createModel(model_t::EModelType modelType, features.push_back(model_t::E_IndividualMaxByPerson); factory->features(features); - gatherer.reset(factory->makeDataGatherer(firstTime)); + gatherer = factory->makeDataGatherer(firstTime); std::shared_ptr model_ = std::make_shared( factory->modelParams(), gatherer, From ef8118339323f1cc76d380f8696fb35d790a5674 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 16:06:01 +0200 Subject: [PATCH 18/21] clang-tidy CDataGatherer --- include/model/CDataGatherer.h | 10 ++--- lib/model/CDataGatherer.cc | 72 +++++++++++++++++------------------ 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/include/model/CDataGatherer.h b/include/model/CDataGatherer.h index dabe633b8..6dd202885 100644 --- a/include/model/CDataGatherer.h +++ b/include/model/CDataGatherer.h @@ -154,7 +154,7 @@ class MODEL_EXPORT CDataGatherer { CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& partitionFieldValue, + std::string partitionFieldValue, const CSearchKey& key, const TFeatureVec& features, const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData); @@ -163,7 +163,7 @@ class MODEL_EXPORT CDataGatherer { CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& partitionFieldValue, + std::string partitionFieldValue, const CSearchKey& key, const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser); @@ -624,9 +624,9 @@ class MODEL_EXPORT CDataGatherer { //! Helper to avoid code duplication when getting a count from a //! field. Logs different errors for missing value and invalid value. - bool extractCountFromField(const std::string& fieldName, - const std::string* fieldValue, - std::size_t& count) const; + static bool extractCountFromField(const std::string& fieldName, + const std::string* fieldValue, + std::size_t& count); //! Helper to avoid code duplication when getting a metric value from a //! field. Logs different errors for missing value and invalid value. diff --git a/lib/model/CDataGatherer.cc b/lib/model/CDataGatherer.cc index f52e3640c..182248e34 100644 --- a/lib/model/CDataGatherer.cc +++ b/lib/model/CDataGatherer.cc @@ -28,6 +28,7 @@ #include #include +#include namespace ml { namespace model { @@ -145,14 +146,14 @@ const std::size_t CDataGatherer::ESTIMATED_MEM_USAGE_PER_OVER_FIELD(1000); CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& partitionFieldValue, + std::string partitionFieldValue, const CSearchKey& key, const TFeatureVec& features, const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData) : m_GathererType(gathererType), m_Features(detail::sanitize(features, gathererType)), m_SummaryMode(summaryMode), m_Params(modelParams), m_SearchKey(key), - m_PartitionFieldValue(partitionFieldValue), + m_PartitionFieldValue(std::move(partitionFieldValue)), m_PeopleRegistry(PERSON, counter_t::E_TSADNumberNewPeople, counter_t::E_TSADNumberNewPeopleNotAllowed, @@ -170,12 +171,12 @@ CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, CDataGatherer::CDataGatherer(model_t::EAnalysisCategory gathererType, model_t::ESummaryMode summaryMode, const SModelParams& modelParams, - const std::string& partitionFieldValue, + std::string partitionFieldValue, const CSearchKey& key, const CBucketGatherer::SBucketGathererInitData& bucketGathererInitData, core::CStateRestoreTraverser& traverser) : m_GathererType(gathererType), m_SummaryMode(summaryMode), m_Params(modelParams), - m_SearchKey(key), m_PartitionFieldValue(partitionFieldValue), + m_SearchKey(key), m_PartitionFieldValue(std::move(partitionFieldValue)), m_PeopleRegistry(PERSON, counter_t::E_TSADNumberNewPeople, counter_t::E_TSADNumberNewPeopleNotAllowed, @@ -210,8 +211,7 @@ CDataGatherer::CDataGatherer(bool isForPersistence, const CDataGatherer& other) } } -CDataGatherer::~CDataGatherer() { -} +CDataGatherer::~CDataGatherer() = default; CDataGatherer* CDataGatherer::cloneForPersistence() const { return new CDataGatherer(true, *this); @@ -296,7 +296,7 @@ bool CDataGatherer::addArrival(const TStrCPtrVec& fieldValues, // the number of partitions created. m_BucketGatherer->processFields(fieldValues, data, resourceMonitor); - core_t::TTime time = data.time(); + core_t::TTime const time = data.time(); if (time < m_BucketGatherer->earliestBucketStartTime()) { // Ignore records that are out of the latency window. // Records in an incomplete first bucket will end up here, @@ -356,7 +356,7 @@ const std::string& CDataGatherer::personName(std::size_t pid, const std::string& } void CDataGatherer::personNonZeroCounts(core_t::TTime time, TSizeUInt64PrVec& result) const { - return m_BucketGatherer->personNonZeroCounts(time, result); + m_BucketGatherer->personNonZeroCounts(time, result); } void CDataGatherer::recyclePeople(const TSizeVec& peopleToRemove) { @@ -474,19 +474,17 @@ std::size_t CDataGatherer::addAttribute(const std::string& attribute, double CDataGatherer::sampleCount(std::size_t id) const { if (m_SampleCounts) { return static_cast(m_SampleCounts->count(id)); - } else { - LOG_ERROR(<< "Sample count for non-metric gatherer"); - return 0.0; } + LOG_ERROR(<< "Sample count for non-metric gatherer"); + return 0.0; } double CDataGatherer::effectiveSampleCount(std::size_t id) const { if (m_SampleCounts) { return m_SampleCounts->effectiveSampleCount(id); - } else { - LOG_ERROR(<< "Effective sample count for non-metric gatherer"); - return 0.0; } + LOG_ERROR(<< "Effective sample count for non-metric gatherer"); + return 0.0; } void CDataGatherer::resetSampleCount(std::size_t id) { @@ -596,24 +594,25 @@ const SModelParams& CDataGatherer::params() const { } void CDataGatherer::acceptPersistInserter(core::CStatePersistInserter& inserter) const { - for (std::size_t i = 0; i < m_Features.size(); ++i) { - inserter.insertValue(FEATURE_TAG, static_cast(m_Features[i])); + for (auto m_Feature : m_Features) { + inserter.insertValue(FEATURE_TAG, static_cast(m_Feature)); } - inserter.insertLevel(PEOPLE_REGISTRY_TAG, - std::bind(&CDynamicStringIdRegistry::acceptPersistInserter, - m_PeopleRegistry, std::placeholders::_1)); - inserter.insertLevel(ATTRIBUTES_REGISTRY_TAG, - std::bind(&CDynamicStringIdRegistry::acceptPersistInserter, - m_AttributesRegistry, std::placeholders::_1)); + inserter.insertLevel(PEOPLE_REGISTRY_TAG, [this](auto&& PH1) { + m_PeopleRegistry.acceptPersistInserter(std::forward(PH1)); + }); + inserter.insertLevel(ATTRIBUTES_REGISTRY_TAG, [this](auto&& PH1) { + m_AttributesRegistry.acceptPersistInserter(std::forward(PH1)); + }); if (m_SampleCounts) { - inserter.insertLevel(SAMPLE_COUNTS_TAG, - std::bind(&CSampleCounts::acceptPersistInserter, - m_SampleCounts.get(), std::placeholders::_1)); + inserter.insertLevel(SAMPLE_COUNTS_TAG, [capture0 = m_SampleCounts.get()](auto&& PH1) { + capture0->acceptPersistInserter(std::forward(PH1)); + }); } - inserter.insertLevel(BUCKET_GATHERER_TAG, std::bind(&CDataGatherer::persistBucketGatherers, - this, std::placeholders::_1)); + inserter.insertLevel(BUCKET_GATHERER_TAG, [this](auto&& inserter_) { + persistBucketGatherers(std::forward(inserter_)); + }); } bool CDataGatherer::determineMetricCategory(TMetricCategoryVec& fieldMetricCategories) const { @@ -642,7 +641,7 @@ bool CDataGatherer::determineMetricCategory(TMetricCategoryVec& fieldMetricCateg bool CDataGatherer::extractCountFromField(const std::string& fieldName, const std::string* fieldValue, - std::size_t& count) const { + std::size_t& count) { if (fieldValue == nullptr) { // Treat not present as explicit null count = EXPLICIT_NULL_SUMMARY_COUNT; @@ -683,13 +682,13 @@ bool CDataGatherer::extractMetricFromField(const std::string& fieldName, // Split the string up by the delimiter and parse each token separately. std::size_t first = 0; do { - std::size_t last = fieldValue.find(delimiter, first); + std::size_t const last = fieldValue.find(delimiter, first); double value; // Avoid a string duplication in the (common) case of only one value - bool convertedOk = (first == 0 && last == std::string::npos) - ? core::CStringUtils::stringToType(fieldValue, value) - : core::CStringUtils::stringToType( - fieldValue.substr(first, last - first), value); + bool const convertedOk = (first == 0 && last == std::string::npos) + ? core::CStringUtils::stringToType(fieldValue, value) + : core::CStringUtils::stringToType( + fieldValue.substr(first, last - first), value); if (!convertedOk) { LOG_ERROR(<< "Unable to extract " << fieldName << " from " << fieldValue); result.clear(); @@ -795,9 +794,10 @@ bool CDataGatherer::restoreBucketGatherer(const CBucketGatherer::SBucketGatherer } void CDataGatherer::persistBucketGatherers(core::CStatePersistInserter& inserter) const { - inserter.insertLevel(m_BucketGatherer->persistenceTag(), - std::bind(&CBucketGatherer::acceptPersistInserter, - m_BucketGatherer.get(), std::placeholders::_1)); + inserter.insertLevel( + m_BucketGatherer->persistenceTag(), [capture0 = m_BucketGatherer.get()](auto&& PH1) { + capture0->acceptPersistInserter(std::forward(PH1)); + }); } void CDataGatherer::createBucketGatherer(model_t::EAnalysisCategory gathererType, From 335042bc49d25b90b0b3a8c04edd4185e0be3efe Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 16:07:57 +0200 Subject: [PATCH 19/21] clang-tidy CBucketGatherer --- lib/model/CBucketGatherer.cc | 78 +++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index 8dabad278..1577c7a36 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -108,10 +108,10 @@ bool restoreInfluencerPersonAttributeCounts(core::CStateRestoreTraverser& traver TSizeSizePrOptionalStrPrUInt64UMap& map) { std::size_t person = 0; std::size_t attribute = 0; - std::string influence = ""; + std::string influence; std::uint64_t count = 0; do { - const std::string name = traverser.name(); + const std::string& name = traverser.name(); RESTORE_BUILT_IN(PERSON_UID_TAG, person) RESTORE_BUILT_IN(ATTRIBUTE_UID_TAG, attribute) RESTORE_NO_ERROR(INFLUENCER_TAG, influence = traverser.value()) @@ -137,10 +137,12 @@ struct SBucketCountsPersister { personAttributeCounts.assign(bucketCounts.begin(), bucketCounts.end()); std::sort(personAttributeCounts.begin(), personAttributeCounts.end()); for (std::size_t i = 0; i < personAttributeCounts.size(); ++i) { - inserter.insertLevel(PERSON_ATTRIBUTE_COUNT_TAG, - std::bind(&insertPersonAttributeCounts, - std::cref(personAttributeCounts[i]), - std::placeholders::_1)); + inserter.insertLevel( + PERSON_ATTRIBUTE_COUNT_TAG, + [capture0 = std::cref(personAttributeCounts[i])](auto&& PH1) { + insertPersonAttributeCounts(capture0, + std::forward(PH1)); + }); } } @@ -148,13 +150,14 @@ struct SBucketCountsPersister { core::CStateRestoreTraverser& traverser) { do { TSizeSizePr key; - std::uint64_t count{0u}; + std::uint64_t count{0U}; if (!traverser.hasSubLevel()) { continue; } - if (traverser.traverseSubLevel( - std::bind(&restorePersonAttributeCounts, std::placeholders::_1, - std::ref(key), std::ref(count))) == false) { + if (traverser.traverseSubLevel([&key, &count](auto&& PH1) { + return restorePersonAttributeCounts( + std::forward(PH1), key, count); + }) == false) { LOG_ERROR(<< "Invalid person attribute count"); continue; } @@ -173,8 +176,10 @@ struct SInfluencerCountsPersister { for (std::size_t i = 0; i < data.size(); ++i) { inserter.insertValue(INFLUENCE_COUNT_TAG, i); inserter.insertLevel(INFLUENCE_ITEM_TAG, - std::bind(&insertInfluencerPersonAttributeCounts, - std::cref(data[i]), std::placeholders::_1)); + [capture0 = std::cref(data[i])](auto&& PH1) { + insertInfluencerPersonAttributeCounts( + capture0, std::forward(PH1)); + }); } } @@ -232,7 +237,7 @@ CBucketGatherer::CBucketGatherer(bool isForPersistence, const CBucketGatherer& o } bool CBucketGatherer::addEventData(CEventData& data) { - core_t::TTime time = data.time(); + core_t::TTime const time = data.time(); if (time < this->earliestBucketStartTime()) { // Ignore records that are out of the latency window @@ -248,9 +253,9 @@ bool CBucketGatherer::addEventData(CEventData& data) { return false; } - std::size_t pid = *data.personId(); - std::size_t cid = *data.attributeId(); - std::size_t count = *data.count(); + std::size_t const pid = *data.personId(); + std::size_t const cid = *data.attributeId(); + std::size_t const count = *data.count(); if ((pid != CDynamicStringIdRegistry::INVALID_ID) && (cid != CDynamicStringIdRegistry::INVALID_ID)) { // Has the person/attribute been deleted from the gatherer? @@ -263,7 +268,7 @@ bool CBucketGatherer::addEventData(CEventData& data) { return false; } - TSizeSizePr pidCid = std::make_pair(pid, cid); + TSizeSizePr const pidCid = std::make_pair(pid, cid); // If record is explicit null just note that a null record has been seen // for the given (pid, cid) pair. @@ -298,7 +303,7 @@ bool CBucketGatherer::addEventData(CEventData& data) { influencerCounts[i] .emplace(boost::unordered::piecewise_construct, boost::make_tuple(pidCid, inf), - boost::make_tuple(std::uint64_t(0))) + boost::make_tuple(static_cast(0))) .first->second += count; } } @@ -316,7 +321,7 @@ void CBucketGatherer::timeNow(core_t::TTime time) { void CBucketGatherer::hiddenTimeNow(core_t::TTime time, bool skipUpdates) { m_EarliestTime = std::min(m_EarliestTime, time); - core_t::TTime n = (time - m_BucketStart) / this->bucketLength(); + core_t::TTime const n = (time - m_BucketStart) / this->bucketLength(); core_t::TTime newBucketStart = m_BucketStart; for (core_t::TTime i = 0; i < n; ++i) { newBucketStart += this->bucketLength(); @@ -325,7 +330,8 @@ void CBucketGatherer::hiddenTimeNow(core_t::TTime time, bool skipUpdates) { // the gatherers may finalise the earliest bucket within // the latency window, thus we push a new count bucket only // after startNewBucket has been called. - std::ptrdiff_t numberInfluences{this->endInfluencers() - this->beginInfluencers()}; + std::ptrdiff_t const numberInfluences{this->endInfluencers() - + this->beginInfluencers()}; this->startNewBucket(newBucketStart, skipUpdates); m_PersonAttributeCounts.push(TSizeSizePrUInt64UMap(1), newBucketStart); m_PersonAttributeExplicitNulls.push(TSizeSizePrUSet(1), newBucketStart); @@ -336,17 +342,17 @@ void CBucketGatherer::hiddenTimeNow(core_t::TTime time, bool skipUpdates) { } void CBucketGatherer::sampleNow(core_t::TTime sampleBucketStart) { - core_t::TTime timeNow = + core_t::TTime const timeNow = sampleBucketStart + - (m_DataGatherer.params().s_LatencyBuckets + 1) * this->bucketLength() - 1; + ((m_DataGatherer.params().s_LatencyBuckets + 1) * this->bucketLength()) - 1; this->timeNow(timeNow); this->sample(sampleBucketStart); } void CBucketGatherer::skipSampleNow(core_t::TTime sampleBucketStart) { - core_t::TTime timeNow = + core_t::TTime const timeNow = sampleBucketStart + - (m_DataGatherer.params().s_LatencyBuckets + 1) * this->bucketLength() - 1; + ((m_DataGatherer.params().s_LatencyBuckets + 1) * this->bucketLength()) - 1; this->hiddenTimeNow(timeNow, true); } @@ -381,7 +387,7 @@ void CBucketGatherer::recyclePeople(const TSizeVec& peopleToRemove) { void CBucketGatherer::removePeople(std::size_t lowestPersonToRemove) { if (lowestPersonToRemove < m_DataGatherer.numberPeople()) { TSizeVec peopleToRemove; - std::size_t maxPersonId = m_DataGatherer.numberPeople(); + std::size_t const maxPersonId = m_DataGatherer.numberPeople(); peopleToRemove.reserve(maxPersonId - lowestPersonToRemove); for (std::size_t pid = lowestPersonToRemove; pid < maxPersonId; ++pid) { peopleToRemove.push_back(pid); @@ -494,7 +500,7 @@ bool CBucketGatherer::hasExplicitNullsOnly(core_t::TTime time, std::size_t pid, return false; } const TSizeSizePrUInt64UMap& bucketCounts = m_PersonAttributeCounts.get(time); - TSizeSizePr pidCid = std::make_pair(pid, cid); + TSizeSizePr const pidCid = std::make_pair(pid, cid); return bucketExplicitNulls.find(pidCid) != bucketExplicitNulls.end() && bucketCounts.find(pidCid) == bucketCounts.end(); } @@ -514,10 +520,10 @@ std::uint64_t CBucketGatherer::checksum() const { TStrCRefStrCRefPrUInt64PrVec personAttributeCounts; personAttributeCounts.reserve(bucketCounts.size()); for (const auto& count : bucketCounts) { - std::size_t pid = CDataGatherer::extractPersonId(count); - std::size_t cid = CDataGatherer::extractAttributeId(count); - TStrCRefStrCRefPr key(TStrCRef(m_DataGatherer.personName(pid)), - TStrCRef(m_DataGatherer.attributeName(cid))); + std::size_t const pid = CDataGatherer::extractPersonId(count); + std::size_t const cid = CDataGatherer::extractAttributeId(count); + TStrCRefStrCRefPr const key(TStrCRef(m_DataGatherer.personName(pid)), + TStrCRef(m_DataGatherer.attributeName(cid))); personAttributeCounts.emplace_back(key, CDataGatherer::extractData(count)); } std::sort(personAttributeCounts.begin(), personAttributeCounts.end(), @@ -531,10 +537,10 @@ std::uint64_t CBucketGatherer::checksum() const { TStrCRefStrCRefPrVec personAttributeExplicitNulls; personAttributeExplicitNulls.reserve(bucketExplicitNulls.size()); for (const auto& nulls : bucketExplicitNulls) { - std::size_t pid = CDataGatherer::extractPersonId(nulls); - std::size_t cid = CDataGatherer::extractAttributeId(nulls); - TStrCRefStrCRefPr key(TStrCRef(m_DataGatherer.personName(pid)), - TStrCRef(m_DataGatherer.attributeName(cid))); + std::size_t const pid = CDataGatherer::extractPersonId(nulls); + std::size_t const cid = CDataGatherer::extractAttributeId(nulls); + TStrCRefStrCRefPr const key(TStrCRef(m_DataGatherer.personName(pid)), + TStrCRef(m_DataGatherer.attributeName(cid))); personAttributeExplicitNulls.push_back(key); } std::sort(personAttributeExplicitNulls.begin(), @@ -583,7 +589,7 @@ bool CBucketGatherer::resetBucket(core_t::TTime bucketStart) { } LOG_TRACE(<< "Resetting bucket starting at " << bucketStart); - std::ptrdiff_t numberInfluences{this->endInfluencers() - this->beginInfluencers()}; + std::ptrdiff_t const numberInfluences{this->endInfluencers() - this->beginInfluencers()}; m_PersonAttributeCounts.get(bucketStart).clear(); m_PersonAttributeExplicitNulls.get(bucketStart).clear(); m_InfluencerCounts.get(bucketStart) = TSizeSizePrOptionalStrPrUInt64UMapVec(numberInfluences); @@ -597,7 +603,7 @@ void CBucketGatherer::baseAcceptPersistInserter(core::CStatePersistInserter& ins std::bind(TSizeSizePrUInt64UMapQueue::CSerializer(), std::cref(m_PersonAttributeCounts), std::placeholders::_1)); // Clear any empty collections before persist these are resized on restore. - TSizeSizePrOptionalStrPrUInt64UMapVecQueue influencerCounts{m_InfluencerCounts}; + TSizeSizePrOptionalStrPrUInt64UMapVecQueue const influencerCounts{m_InfluencerCounts}; inserter.insertLevel( INFLUENCERS_COUNT_TAG, std::bind(TSizeSizePrOptionalStrPrUInt64UMapVecQueue::CSerializer(), From a843fd1f2ac36e39d3115c9db3a1e233dc5bf413 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Thu, 10 Apr 2025 16:08:26 +0200 Subject: [PATCH 20/21] formatting --- lib/model/CBucketGatherer.cc | 14 +++++++------- lib/model/CCountingModelFactory.cc | 12 ++++++------ lib/model/CEventRateModelFactory.cc | 12 ++++++------ lib/model/CEventRatePopulationModelFactory.cc | 13 +++++++------ lib/model/CMetricModelFactory.cc | 12 ++++++------ lib/model/CMetricPopulationModelFactory.cc | 13 +++++++------ 6 files changed, 39 insertions(+), 37 deletions(-) diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index 1577c7a36..85ffc9138 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -138,8 +138,8 @@ struct SBucketCountsPersister { std::sort(personAttributeCounts.begin(), personAttributeCounts.end()); for (std::size_t i = 0; i < personAttributeCounts.size(); ++i) { inserter.insertLevel( - PERSON_ATTRIBUTE_COUNT_TAG, - [capture0 = std::cref(personAttributeCounts[i])](auto&& PH1) { + PERSON_ATTRIBUTE_COUNT_TAG, [capture0 = std::cref( + personAttributeCounts[i])](auto&& PH1) { insertPersonAttributeCounts(capture0, std::forward(PH1)); }); @@ -175,11 +175,11 @@ struct SInfluencerCountsPersister { core::CStatePersistInserter& inserter) { for (std::size_t i = 0; i < data.size(); ++i) { inserter.insertValue(INFLUENCE_COUNT_TAG, i); - inserter.insertLevel(INFLUENCE_ITEM_TAG, - [capture0 = std::cref(data[i])](auto&& PH1) { - insertInfluencerPersonAttributeCounts( - capture0, std::forward(PH1)); - }); + inserter.insertLevel( + INFLUENCE_ITEM_TAG, [capture0 = std::cref(data[i])](auto&& PH1) { + insertInfluencerPersonAttributeCounts( + capture0, std::forward(PH1)); + }); } } diff --git a/lib/model/CCountingModelFactory.cc b/lib/model/CCountingModelFactory.cc index 3f4c652da..fe0238a14 100644 --- a/lib/model/CCountingModelFactory.cc +++ b/lib/model/CCountingModelFactory.cc @@ -69,9 +69,9 @@ CCountingModelFactory::makeDataGatherer(const SGathererInitializationData& initD {}, initData.s_StartTime, 0}; - return std::make_shared(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), initData.s_PartitionFieldValue, - this->searchKey(), m_Features, bucketGathererInitData); + return std::make_shared( + model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CModelFactory::TDataGathererPtr @@ -79,9 +79,9 @@ CCountingModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, EMPTY_STRING, {}, 0, 0}; - return std::make_shared(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), partitionFieldValue, - this->searchKey(), bucketGathererInitData, traverser); + return std::make_shared( + model_t::E_EventRate, m_SummaryMode, this->modelParams(), + partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } CCountingModelFactory::TPriorPtr diff --git a/lib/model/CEventRateModelFactory.cc b/lib/model/CEventRateModelFactory.cc index 579ccd998..2c2947bd8 100644 --- a/lib/model/CEventRateModelFactory.cc +++ b/lib/model/CEventRateModelFactory.cc @@ -100,9 +100,9 @@ CEventRateModelFactory::makeDataGatherer(const SGathererInitializationData& init m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount}; - return std::make_shared(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), initData.s_PartitionFieldValue, - this->searchKey(), m_Features, bucketGathererInitData); + return std::make_shared( + model_t::E_EventRate, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CModelFactory::TDataGathererPtr @@ -110,9 +110,9 @@ CEventRateModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; - return std::make_shared(model_t::E_EventRate, m_SummaryMode, - this->modelParams(), partitionFieldValue, - this->searchKey(), bucketGathererInitData, traverser); + return std::make_shared( + model_t::E_EventRate, m_SummaryMode, this->modelParams(), + partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } CEventRateModelFactory::TPriorPtr diff --git a/lib/model/CEventRatePopulationModelFactory.cc b/lib/model/CEventRatePopulationModelFactory.cc index cbda1921b..1c3836c00 100644 --- a/lib/model/CEventRatePopulationModelFactory.cc +++ b/lib/model/CEventRatePopulationModelFactory.cc @@ -101,9 +101,10 @@ CEventRatePopulationModelFactory::makeDataGatherer(const SGathererInitialization .s_InfluenceFieldNames = m_InfluenceFieldNames, .s_StartTime = initData.s_StartTime, .s_SampleOverrideCount = 0}; - return std::make_shared(model_t::E_PopulationEventRate, m_SummaryMode, - this->modelParams(), initData.s_PartitionFieldValue, - this->searchKey(), m_Features, bucketGathererInitData); + return std::make_shared( + model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), + initData.s_PartitionFieldValue, this->searchKey(), m_Features, + bucketGathererInitData); } CModelFactory::TDataGathererPtr @@ -117,9 +118,9 @@ CEventRatePopulationModelFactory::makeDataGatherer(const std::string& partitionF .s_InfluenceFieldNames = m_InfluenceFieldNames, .s_StartTime = 0, .s_SampleOverrideCount = 0}; - return std::make_shared(model_t::E_PopulationEventRate, m_SummaryMode, - this->modelParams(), partitionFieldValue, - this->searchKey(), bucketGathererInitData, traverser); + return std::make_shared( + model_t::E_PopulationEventRate, m_SummaryMode, this->modelParams(), + partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } CEventRatePopulationModelFactory::TPriorPtr diff --git a/lib/model/CMetricModelFactory.cc b/lib/model/CMetricModelFactory.cc index 4559dffaf..f74f2ce3b 100644 --- a/lib/model/CMetricModelFactory.cc +++ b/lib/model/CMetricModelFactory.cc @@ -100,9 +100,9 @@ CMetricModelFactory::makeDataGatherer(const SGathererInitializationData& initDat m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount}; - return std::make_shared(model_t::E_Metric, m_SummaryMode, this->modelParams(), - initData.s_PartitionFieldValue, this->searchKey(), - m_Features, bucketGathererInitData); + return std::make_shared( + model_t::E_Metric, m_SummaryMode, this->modelParams(), initData.s_PartitionFieldValue, + this->searchKey(), m_Features, bucketGathererInitData); } CModelFactory::TDataGathererPtr @@ -110,9 +110,9 @@ CMetricModelFactory::makeDataGatherer(const std::string& partitionFieldValue, core::CStateRestoreTraverser& traverser) const { CBucketGatherer::SBucketGathererInitData bucketGathererInitData{ m_SummaryCountFieldName, m_PersonFieldName, EMPTY_STRING, m_ValueFieldName, m_InfluenceFieldNames, 0, 0}; - return std::make_shared(model_t::E_Metric, m_SummaryMode, - this->modelParams(), partitionFieldValue, - this->searchKey(), bucketGathererInitData, traverser); + return std::make_shared( + model_t::E_Metric, m_SummaryMode, this->modelParams(), partitionFieldValue, + this->searchKey(), bucketGathererInitData, traverser); } CMetricModelFactory::TPriorPtr diff --git a/lib/model/CMetricPopulationModelFactory.cc b/lib/model/CMetricPopulationModelFactory.cc index b601284cf..deff0c863 100644 --- a/lib/model/CMetricPopulationModelFactory.cc +++ b/lib/model/CMetricPopulationModelFactory.cc @@ -97,9 +97,10 @@ CMetricPopulationModelFactory::makeDataGatherer(const SGathererInitializationDat m_AttributeFieldName, m_ValueFieldName, m_InfluenceFieldNames, initData.s_StartTime, initData.s_SampleOverrideCount}; - return std::make_shared(model_t::E_PopulationMetric, m_SummaryMode, - this->modelParams(), initData.s_PartitionFieldValue, - this->searchKey(), m_Features, bucketGathererInitData); + return std::make_shared( + model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), + initData.s_PartitionFieldValue, this->searchKey(), m_Features, + bucketGathererInitData); } CModelFactory::TDataGathererPtr @@ -113,9 +114,9 @@ CMetricPopulationModelFactory::makeDataGatherer(const std::string& partitionFiel m_InfluenceFieldNames, 0, 0}; - return std::make_shared(model_t::E_PopulationMetric, m_SummaryMode, - this->modelParams(), partitionFieldValue, - this->searchKey(), bucketGathererInitData, traverser); + return std::make_shared( + model_t::E_PopulationMetric, m_SummaryMode, this->modelParams(), + partitionFieldValue, this->searchKey(), bucketGathererInitData, traverser); } CMetricPopulationModelFactory::TPriorPtr From a77bc90c8cbb30fe2dd9569b2408ef390a4186f4 Mon Sep 17 00:00:00 2001 From: Valeriy Khakhutskyy <1292899+valeriy42@users.noreply.github.com> Date: Fri, 11 Apr 2025 09:25:54 +0200 Subject: [PATCH 21/21] linting --- lib/model/CBucketGatherer.cc | 21 +++++++++------------ lib/model/CDataGatherer.cc | 26 ++++++++++++++------------ 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/model/CBucketGatherer.cc b/lib/model/CBucketGatherer.cc index 85ffc9138..1c59ddd44 100644 --- a/lib/model/CBucketGatherer.cc +++ b/lib/model/CBucketGatherer.cc @@ -138,10 +138,9 @@ struct SBucketCountsPersister { std::sort(personAttributeCounts.begin(), personAttributeCounts.end()); for (std::size_t i = 0; i < personAttributeCounts.size(); ++i) { inserter.insertLevel( - PERSON_ATTRIBUTE_COUNT_TAG, [capture0 = std::cref( - personAttributeCounts[i])](auto&& PH1) { - insertPersonAttributeCounts(capture0, - std::forward(PH1)); + PERSON_ATTRIBUTE_COUNT_TAG, [tuple = std::cref(personAttributeCounts[i])]( + core::CStatePersistInserter & inserter_) { + insertPersonAttributeCounts(tuple, inserter_); }); } } @@ -154,9 +153,8 @@ struct SBucketCountsPersister { if (!traverser.hasSubLevel()) { continue; } - if (traverser.traverseSubLevel([&key, &count](auto&& PH1) { - return restorePersonAttributeCounts( - std::forward(PH1), key, count); + if (traverser.traverseSubLevel([&key, &count](core::CStateRestoreTraverser& traverser_) { + return restorePersonAttributeCounts(traverser_, key, count); }) == false) { LOG_ERROR(<< "Invalid person attribute count"); continue; @@ -175,11 +173,10 @@ struct SInfluencerCountsPersister { core::CStatePersistInserter& inserter) { for (std::size_t i = 0; i < data.size(); ++i) { inserter.insertValue(INFLUENCE_COUNT_TAG, i); - inserter.insertLevel( - INFLUENCE_ITEM_TAG, [capture0 = std::cref(data[i])](auto&& PH1) { - insertInfluencerPersonAttributeCounts( - capture0, std::forward(PH1)); - }); + inserter.insertLevel(INFLUENCE_ITEM_TAG, [map = std::cref(data[i])]( + core::CStatePersistInserter & inserter_) { + insertInfluencerPersonAttributeCounts(map, inserter_); + }); } } diff --git a/lib/model/CDataGatherer.cc b/lib/model/CDataGatherer.cc index 182248e34..dffe6efb8 100644 --- a/lib/model/CDataGatherer.cc +++ b/lib/model/CDataGatherer.cc @@ -296,8 +296,8 @@ bool CDataGatherer::addArrival(const TStrCPtrVec& fieldValues, // the number of partitions created. m_BucketGatherer->processFields(fieldValues, data, resourceMonitor); - core_t::TTime const time = data.time(); - if (time < m_BucketGatherer->earliestBucketStartTime()) { + if (core_t::TTime const time = data.time(); + time < m_BucketGatherer->earliestBucketStartTime()) { // Ignore records that are out of the latency window. // Records in an incomplete first bucket will end up here, // but we don't want to model these. @@ -597,21 +597,22 @@ void CDataGatherer::acceptPersistInserter(core::CStatePersistInserter& inserter) for (auto m_Feature : m_Features) { inserter.insertValue(FEATURE_TAG, static_cast(m_Feature)); } - inserter.insertLevel(PEOPLE_REGISTRY_TAG, [this](auto&& PH1) { - m_PeopleRegistry.acceptPersistInserter(std::forward(PH1)); + inserter.insertLevel(PEOPLE_REGISTRY_TAG, [this](core::CStatePersistInserter& inserter_) { + m_PeopleRegistry.acceptPersistInserter(inserter_); }); - inserter.insertLevel(ATTRIBUTES_REGISTRY_TAG, [this](auto&& PH1) { - m_AttributesRegistry.acceptPersistInserter(std::forward(PH1)); + inserter.insertLevel(ATTRIBUTES_REGISTRY_TAG, [this](core::CStatePersistInserter& inserter_) { + m_AttributesRegistry.acceptPersistInserter(inserter_); }); if (m_SampleCounts) { - inserter.insertLevel(SAMPLE_COUNTS_TAG, [capture0 = m_SampleCounts.get()](auto&& PH1) { - capture0->acceptPersistInserter(std::forward(PH1)); + inserter.insertLevel(SAMPLE_COUNTS_TAG, [sampleCounts = m_SampleCounts.get()]( + core::CStatePersistInserter & inserter_) { + sampleCounts->acceptPersistInserter(inserter_); }); } - inserter.insertLevel(BUCKET_GATHERER_TAG, [this](auto&& inserter_) { - persistBucketGatherers(std::forward(inserter_)); + inserter.insertLevel(BUCKET_GATHERER_TAG, [this](core::CStatePersistInserter& inserter_) { + persistBucketGatherers(inserter_); }); } @@ -795,8 +796,9 @@ bool CDataGatherer::restoreBucketGatherer(const CBucketGatherer::SBucketGatherer void CDataGatherer::persistBucketGatherers(core::CStatePersistInserter& inserter) const { inserter.insertLevel( - m_BucketGatherer->persistenceTag(), [capture0 = m_BucketGatherer.get()](auto&& PH1) { - capture0->acceptPersistInserter(std::forward(PH1)); + m_BucketGatherer->persistenceTag(), [capture0 = m_BucketGatherer.get()]( + core::CStatePersistInserter & inserter_) { + capture0->acceptPersistInserter(inserter_); }); }