From cf12b73ec44cf0c553319d675b780e6e8c84f0bc Mon Sep 17 00:00:00 2001
From: LHesperus <2639350497@qq.com>
Date: Tue, 3 Dec 2024 16:10:46 +0800
Subject: [PATCH 2/4]  fix bug : free spe mem after use data

---
 src/plugin/collect/pmu/pmu_spe_collector.cpp | 66 ++++++++++++--------
 src/plugin/collect/pmu/pmu_spe_collector.h   |  5 +-
 2 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/src/plugin/collect/pmu/pmu_spe_collector.cpp b/src/plugin/collect/pmu/pmu_spe_collector.cpp
index 114c0ee..8d265eb 100644
--- a/src/plugin/collect/pmu/pmu_spe_collector.cpp
+++ b/src/plugin/collect/pmu/pmu_spe_collector.cpp
@@ -63,15 +63,26 @@ int PmuSpeCollector::OpenSpe()
     return pd;
 }
 
-void PmuSpeCollector::DynamicAdjustPeriod(uint64_t interval)
+void PmuSpeCollector::DynamicAdjustPeriod(int interval)
 {
+    if (pmuId == -1) {
+        return;
+    }
     if (interval > timeout) {
         PmuDisable(pmuId);
         PmuClose(pmuId);
         attrPeriod *= periodThreshold;
+        if (attrPeriod > maxAttrPeriod) {
+            attrPeriod = maxAttrPeriod;
+        }
+        INFO(logger, "PmuSpeCollector dynamic adjust period to " <<
+            attrPeriod << ", PmuRead interval is " << interval << " ms.");
         pmuId = OpenSpe();
-        PmuEnable(pmuId);
+        if (pmuId != -1) {
+            PmuEnable(pmuId);
+        }
     }
+    // later add code to decrease period when interval is too small
 }
 
 oeaware::Result PmuSpeCollector::OpenTopic(const oeaware::Topic &topic)
@@ -79,7 +90,8 @@ oeaware::Result PmuSpeCollector::OpenTopic(const oeaware::Topic &topic)
     if (topic.instanceName != this->name || topic.topicName != topicStr) {
         return oeaware::Result(FAILED, "OpenTopic failed");
     }
-
+    attrPeriod = minAttrPeriod;
+    readTimeMs = 0;
     if (pmuId == -1) {
         pmuId = OpenSpe();
         if (pmuId == -1) {
@@ -124,27 +136,31 @@ void PmuSpeCollector::UpdateData(const DataList &dataList)
 
 void PmuSpeCollector::Run()
 {
-    if (pmuId != -1) {
-        PmuSpeData *data = new PmuSpeData();
-        PmuDisable(pmuId);
-        data->len = PmuRead(pmuId, &(data->pmuData));
-        PmuEnable(pmuId);
-
-        auto now = std::chrono::high_resolution_clock::now();
-        data->interval = std::chrono::duration_cast<std::chrono::milliseconds>(now - timestamp).count();
-        DynamicAdjustPeriod(data->interval);
-        timestamp = std::chrono::high_resolution_clock::now();
-
-        DataList dataList;
-        dataList.topic.instanceName = new char[name.size() + 1];
-        strcpy_s(dataList.topic.instanceName, name.size() + 1, name.data());
-        dataList.topic.topicName = new char[topicStr.size() + 1];
-        strcpy_s(dataList.topic.topicName, topicStr.size() + 1, topicStr.data());
-        dataList.topic.params = new char[1];
-        dataList.topic.params[0] = 0;
-        dataList.len = 1;
-        dataList.data = new void* [1];
-        dataList.data[0] = data;
-        Publish(dataList);
+    // adjust period will pmuclose and free spe data
+    // so adjust period should be done after other plugins have finished using SPE data
+    DynamicAdjustPeriod(readTimeMs);
+    if (pmuId == -1) {
+        return;
     }
+    PmuSpeData *data = new PmuSpeData();
+    PmuDisable(pmuId);
+    auto readBegin = std::chrono::high_resolution_clock::now();
+    data->len = PmuRead(pmuId, &(data->pmuData));
+    readTimeMs = std::chrono::duration_cast<std::chrono::milliseconds>(
+        std::chrono::high_resolution_clock::now() - readBegin).count();
+    PmuEnable(pmuId);
+    auto now = std::chrono::high_resolution_clock::now();
+    data->interval = std::chrono::duration_cast<std::chrono::milliseconds>(now - timestamp).count();
+    timestamp = std::chrono::high_resolution_clock::now();
+    DataList dataList;
+    dataList.topic.instanceName = new char[name.size() + 1];
+    strcpy_s(dataList.topic.instanceName, name.size() + 1, name.data());
+    dataList.topic.topicName = new char[topicStr.size() + 1];
+    strcpy_s(dataList.topic.topicName, topicStr.size() + 1, topicStr.data());
+    dataList.topic.params = new char[1];
+    dataList.topic.params[0] = 0;
+    dataList.len = 1;
+    dataList.data = new void *[1];
+    dataList.data[0] = data;
+    Publish(dataList);
 }
\ No newline at end of file
diff --git a/src/plugin/collect/pmu/pmu_spe_collector.h b/src/plugin/collect/pmu/pmu_spe_collector.h
index 00f0be7..3323e47 100644
--- a/src/plugin/collect/pmu/pmu_spe_collector.h
+++ b/src/plugin/collect/pmu/pmu_spe_collector.h
@@ -28,7 +28,7 @@ public:
     void Disable() override;
     void Run() override;
 private:
-    void DynamicAdjustPeriod(uint64_t interval);
+    void DynamicAdjustPeriod(int interval);
     void InitSpeAttr(struct PmuAttr &attr);
     int OpenSpe();
 
@@ -38,6 +38,9 @@ private:
     std::chrono::time_point<std::chrono::high_resolution_clock> timestamp;
     const int timeout = 50;
     const int periodThreshold = 2;
+    const int minAttrPeriod = 2048;
+    const int maxAttrPeriod = 2048000;
+    int readTimeMs = 0; // last period PmuRead() time
 };
 
 #endif
\ No newline at end of file
-- 
2.33.0