2016Q4.feature.odcockpit-performance-monitor-plugin #139

fgiaimo · 2016-12-07T13:14:55Z

Working for latest module started, responsiveness can be improved (later on)

….Q4.feature.odcockpit.performanceMonitor

… 2016.Q4.feature.odcockpit.performanceMonitor

HangYin2015 · 2016-12-08T07:21:27Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+
+                    // The number of processors currently online
+                    int numCPU = sysconf(_SC_NPROCESSORS_ONLN);
+                    if(numCPU == -1) OPENDAVINCI_CORE_THROW_EXCEPTION(IOException,"Could not get system info");


It is better to enclose the exception statement in brackets (the same applies to other if conditions and for loops):
if(numCPU == -1){
...
}

HangYin2015 · 2016-12-08T07:22:32Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                        proc_file>>str;
+                        proc_file>>chr;
+                        for(uint8_t i=0;i<11;++i)
+                            proc_file>>temp_buffer;


It is more readable to put it in brackets

HangYin2015 · 2016-12-08T07:34:39Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                        start_time=(double)temp_buffer/tickspersec;
+                        // Virtual memory size in bytes
+                        proc_file>>temp_buffer;
+                        total_vm_size=temp_buffer;


Why are total_vm_size and resident_set_size of double type? Should they be integers?

They are doubles because they are assigned to the same array together with floating point % variables.
Integer (re-)conversion is performed at a later step.

chrberger · 2016-12-08T09:42:03Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                    // The number of processors currently online
+                    int numCPU = sysconf(_SC_NPROCESSORS_ONLN);
+                    if(numCPU == -1) {
+                        OPENDAVINCI_CORE_THROW_EXCEPTION(IOException,NO_SYS_INFO_EXC_MSG);


We need to be extract careful in this class as all modules depend on its flawless behavior.

I suggest to avoid throwing an exception here but adjust the code in such a way that it can handle ::sysconf(...) calls that might fail or return unexpected values without throwing an exception. @fgiaimo Could you please adjust the code accordingly? Thanks.

Would a silent fail be better? The code can send out dummy values like -1 if there are errors

Or a boolean value is_valid can be added to the final statistics data structures (CPUConsumption and MEMConsumption)

The question is also if this call failed once, wouldn't it fail always? I.e., could you add a flag to avoid re-executing and failing to execute sysconf(...) repeatedly?

chrberger · 2016-12-08T09:42:14Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                    // Size of a page in bytes
+                    int pagesize_in_bytes = sysconf(_SC_PAGESIZE);
+                    if(pagesize_in_bytes < 1) {
+                        OPENDAVINCI_CORE_THROW_EXCEPTION(IOException,NO_SYS_INFO_EXC_MSG);


chrberger · 2016-12-08T09:42:19Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                    // The number of clock ticks per second
+                    long tickspersec = sysconf(_SC_CLK_TCK);
+                    if(tickspersec == -1) {
+                        OPENDAVINCI_CORE_THROW_EXCEPTION(IOException,NO_SYS_INFO_EXC_MSG);


chrberger · 2016-12-08T09:42:45Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                    double utime, stime, start_time, total_vm_size, resident_set_size;
+
+                    ifstream proc_file;
+                    proc_file.open("/proc/self/stat");


Here, we should check if proc_file could successfully be opened.

chrberger · 2016-12-08T09:43:34Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                            proc_file>>temp_buffer;
+                        }
+                        // Amount of time that this process has been scheduled in user mode (in clock ticks)
+                        utime=(double)temp_buffer/tickspersec/numCPU;


tickspersec and numCPU will never be 0?

chrberger · 2016-12-08T09:44:06Z

libopendavinci/src/odcore/base/module/ManagedClientModule.cpp

+                    double cpu_time_delta=cpu_time_now-m_stats_cpu_time;
+                    double exec_time_delta=exec_time_now-m_stats_exc_time;
+                    double avg_cpu=m_stats_cpu_time/m_stats_exc_time*100.0;
+                    double ondemand_cpu=cpu_time_delta/exec_time_delta*100.0;


exec_time_delta will never be 0?

chrberger · 2016-12-08T09:49:36Z

odcockpit/src/plugins/performancemonitor/PerformanceMonitorWidget.cpp

+                    const string subentryCPU = sstr.str();
+                    // search for the avg CPU stats container
+                    if (m_CPUStatisticsPerComponent.find(subentryCPU) == m_CPUStatisticsPerComponent.end()) {
+                        QTreeWidgetItem *newComponentStatisticsHeader = new QTreeWidgetItem();


We won't exhaust all memory here, right?

I assume not, but how do I check for that?

chrberger · 2016-12-08T09:49:50Z

odcockpit/src/plugins/performancemonitor/PerformanceMonitorWidget.cpp

+                    const string subentryCPU = sstr.str();
+                    // search for the od CPU stats container
+                    if (m_CPUStatisticsPerComponent.find(subentryCPU) == m_CPUStatisticsPerComponent.end()) {
+                        QTreeWidgetItem *newComponentStatisticsHeader = new QTreeWidgetItem();


We won't all memory here, right?

chrberger · 2016-12-08T09:50:09Z

odcockpit/src/plugins/performancemonitor/PerformanceMonitorWidget.cpp

+                    const string subentryMEM = sstr.str();
+
+                    if (m_MEMStatisticsPerComponent.find(subentryMEM) == m_MEMStatisticsPerComponent.end()) {
+                        QTreeWidgetItem *newComponentStatisticsHeader = new QTreeWidgetItem();


We won't exhaust all memory here, right?

chrberger · 2016-12-08T09:50:57Z

odcockpit/src/plugins/performancemonitor/PerformanceMonitorWidget.cpp

+                    const string subentryMEM = sstr.str();
+
+                    if (m_MEMStatisticsPerComponent.find(subentryMEM) == m_MEMStatisticsPerComponent.end()) {
+                        QTreeWidgetItem *newComponentStatisticsHeader = new QTreeWidgetItem();


We won't exhaust all memory here, right?

chrberger · 2019-07-24T11:19:03Z

We have moved to libcluon and JS visualizations; is this PR still relevant?

fgiaimo · 2019-07-24T11:43:13Z

I think not, it can be removed.

HangYin2015 and others added 19 commits October 26, 2016 15:50

New PerformanceMonitor plugin based on LogMessage

728dcf3

The framework of the performance monitor plugin is ready

72acaba

Merge branch 'master' of github.com:se-research/OpenDaVINCI into 2016…

eb5dc53

….Q4.feature.odcockpit.performanceMonitor

Plugin ready to show usage values

3ab9960

WIP temporary commit

a43a145

Added data structures for cpu and mem consumption

8945f6a

Functionalities added, preliminary tests are working

dc64409

Merge branch 'master' of github.com:se-research/OpenDaVINCI into 2016…

3fb01b1

….Q4.feature.odcockpit.performanceMonitor

Cleaned up Driver class

ef48a99

Cleaned up ManagedClientModule

e447fd8

Spacing in printed message

d59fbee

Cleaned up supercomponent

1479716

Removed redundant loc

9021e39

Expanded documentation comment

a7aace9

minor style fix

9010ff9

Spacing in printed message -2

3cdefe7

Added optional conf parameters for history (default=last 3 entries)

b3672a1

Merge branch 'pre-release' of github.com:se-research/OpenDaVINCI into…

41ea5a0

… 2016.Q4.feature.odcockpit.performanceMonitor

Removed unused header

f37a1ed

fgiaimo assigned chrberger and HangYin2015 Dec 7, 2016

HangYin2015 reviewed Dec 8, 2016

View reviewed changes

Added brackets

68080f1

HangYin2015 approved these changes Dec 8, 2016

View reviewed changes

chrberger added the ready4merge label Dec 8, 2016

chrberger changed the title ~~odcockpit performance monitor plugin~~ 2016Q4.feature.odcockpit-performance-monitor-plugin Dec 8, 2016

chrberger added toBeReviewed and removed ready4merge labels Dec 8, 2016

chrberger requested changes Dec 8, 2016

View reviewed changes

Added sanity checks

a8de741

chrberger closed this Jul 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2016Q4.feature.odcockpit-performance-monitor-plugin #139

2016Q4.feature.odcockpit-performance-monitor-plugin #139

fgiaimo commented Dec 7, 2016 •

edited

Loading

HangYin2015 Dec 8, 2016 •

edited

Loading

HangYin2015 Dec 8, 2016

HangYin2015 Dec 8, 2016

fgiaimo Dec 8, 2016

chrberger Dec 8, 2016

fgiaimo Dec 8, 2016

fgiaimo Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

fgiaimo Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger Dec 8, 2016

chrberger commented Jul 24, 2019

fgiaimo commented Jul 24, 2019

2016Q4.feature.odcockpit-performance-monitor-plugin #139

2016Q4.feature.odcockpit-performance-monitor-plugin #139

Conversation

fgiaimo commented Dec 7, 2016 • edited Loading

HangYin2015 Dec 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrberger commented Jul 24, 2019

fgiaimo commented Jul 24, 2019

fgiaimo commented Dec 7, 2016 •

edited

Loading

HangYin2015 Dec 8, 2016 •

edited

Loading