From 75a155bfed5cb800c2a4561450aec93b27c3332f Mon Sep 17 00:00:00 2001
From: Chris Jones <jonesc@hep.phy.cam.ac.uk>
Date: Fri, 13 Jan 2023 14:45:37 +0000
Subject: [PATCH] ProcStats : Use mutex lock to make fetch() thread safe.
: Also minor cleanup and general code modernisation.
---
GaudiAud/src/ProcStats.cpp | 56 +++++++++++++--------------
GaudiAud/src/ProcStats.h | 77 ++++++++++++++++++++------------------
2 files changed, 66 insertions(+), 67 deletions(-)
diff --git a/GaudiAud/src/ProcStats.cpp b/GaudiAud/src/ProcStats.cpp
index 92ad303cf4..60fb0e7bd8 100644
--- a/GaudiAud/src/ProcStats.cpp
+++ b/GaudiAud/src/ProcStats.cpp
@@ -18,6 +18,7 @@
#endif
#include "ProcStats.h"
+#include <memory>
#if defined( __linux__ ) or defined( __APPLE__ )
# include <iostream>
@@ -239,55 +240,50 @@ struct linux_proc {
};
#endif // __linux__ or __APPLE__
-ProcStats::cleanup::~cleanup() {
- if ( ProcStats::inst != 0 ) {
- delete ProcStats::inst;
- ProcStats::inst = 0;
- }
-}
-
ProcStats* ProcStats::instance() {
- static cleanup c;
- if ( !inst ) inst = new ProcStats;
- return inst;
+ static std::once_flag alloc_instance_once;
+ static std::unique_ptr<ProcStats> inst;
+ std::call_once( alloc_instance_once, []() { inst = std::make_unique<ProcStats>(); } );
+ return inst.get();
}
-ProcStats* ProcStats::inst = 0;
-
-ProcStats::ProcStats() : valid( false ) {
+ProcStats::ProcStats() {
#if defined( __linux__ ) or defined( __APPLE__ )
- pg_size = sysconf( _SC_PAGESIZE ); // getpagesize();
+ m_pg_size = sysconf( _SC_PAGESIZE ); // getpagesize();
- fname = "/proc/" + std::to_string( getpid() ) + "/stat";
+ m_fname = "/proc/" + std::to_string( getpid() ) + "/stat";
- fd.open( fname.c_str(), O_RDONLY );
- if ( !fd ) {
- cerr << "Failed to open " << fname << endl;
+ m_ufd.open( m_fname.c_str(), O_RDONLY );
+ if ( !m_ufd ) {
+ cerr << "Failed to open " << m_fname << endl;
return;
}
#endif // __linux__ or __APPLE__
- valid = true;
+ m_valid = true;
}
bool ProcStats::fetch( procInfo& f ) {
- if ( valid == false ) return false;
+ if ( !m_valid ) { return false; }
#if defined( __linux__ ) or defined( __APPLE__ )
- double pr_size, pr_rssize;
+
+ std::scoped_lock lock{ m_mutex };
+
+ double pr_size{ 0 }, pr_rssize{ 0 };
linux_proc pinfo;
- int cnt;
+ int cnt{ 0 };
- fd.lseek( 0, SEEK_SET );
+ m_ufd.lseek( 0, SEEK_SET );
- if ( ( cnt = fd.read( buf, sizeof( buf ) ) ) < 0 ) {
+ if ( ( cnt = m_ufd.read( m_buf, sizeof( m_buf ) ) ) < 0 ) {
cout << "LINUX Read of Proc file failed:" << endl;
return false;
}
if ( cnt > 0 ) {
- buf[std::min( static_cast<std::size_t>( cnt ), sizeof( buf ) - 1 )] = '\0';
+ m_buf[std::min( static_cast<std::size_t>( cnt ), sizeof( m_buf ) - 1 )] = '\0';
- sscanf( buf,
+ sscanf( m_buf,
// 1 2 3 4 5 6 7 8 9 10 1 2 3 4 5 6 7 8 9 20 1 2 3 4 5 6 7 8 9
// 30 1 2 3 4 5
"%d %s %c %d %d %d %d %d %lu %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld %ld %ld %llu %lu %ld %lu %lu %lu %lu "
@@ -304,7 +300,7 @@ bool ProcStats::fetch( procInfo& f ) {
pr_rssize = (double)pinfo.rss;
f.vsize = pr_size / ( 1024 * 1024 );
- f.rss = pr_rssize * pg_size / ( 1024 * 1024 );
+ f.rss = pr_rssize * m_pg_size / ( 1024 * 1024 );
}
#else
@@ -312,10 +308,10 @@ bool ProcStats::fetch( procInfo& f ) {
f.rss = 0;
#endif // __linux__ or __APPLE__
- bool rc = ( curr == f ) ? false : true;
+ const bool rc = !( m_curr == f );
- curr.rss = f.rss;
- curr.vsize = f.vsize;
+ m_curr.rss = f.rss;
+ m_curr.vsize = f.vsize;
return rc;
}
diff --git a/GaudiAud/src/ProcStats.h b/GaudiAud/src/ProcStats.h
index 1127540822..067c9e4173 100644
--- a/GaudiAud/src/ProcStats.h
+++ b/GaudiAud/src/ProcStats.h
@@ -8,15 +8,17 @@
* granted to it by virtue of its status as an Intergovernmental Organization *
* or submit itself to any jurisdiction. *
\***********************************************************************************/
-#ifndef GAUDIAUD_PROCSTATS_H
-#define GAUDIAUD_PROCSTATS_H
+
+#pragma once
// Class: ProcStats
// Description: Keeps statistics on memory usage
// Author: Jim Kowalkowski (FNAL), modified by M. Shapiro (LBNL)
+#include <mutex>
#include <string>
#include <vector>
+
#if defined( __linux__ ) or defined( __APPLE__ )
# include <fcntl.h>
# include <sys/stat.h>
@@ -25,7 +27,7 @@
#endif // __linux__ or __APPLE__
struct procInfo {
- procInfo() : vsize( 0 ), rss( 0 ) {}
+ procInfo() = default;
procInfo( double sz, double rss_sz ) : vsize( sz ), rss( rss_sz ) {}
bool operator==( const procInfo& p ) const {
@@ -35,7 +37,7 @@ struct procInfo {
# pragma warning( disable : 1572 )
#endif
- return vsize == p.vsize && rss == p.rss;
+ return ( vsize == p.vsize && rss == p.rss );
#ifdef __ICC
// re-enable icc remark #1572
@@ -44,29 +46,23 @@ struct procInfo {
}
// see proc(4) man pages for units and a description
- double vsize; // in MB (used to be in pages?)
- double rss; // in MB (used to be in pages?)
+ double vsize{ 0 }; // in MB (used to be in pages?)
+ double rss{ 0 }; // in MB (used to be in pages?)
};
class ProcStats {
public:
static ProcStats* instance();
- bool fetch( procInfo& fill_me );
- double pageSize() const { return pg_size; }
-
-private:
+ bool fetch( procInfo& fill_me );
+ auto pageSize() const noexcept { return m_pg_size; }
ProcStats();
- struct cleanup {
- cleanup() {}
- ~cleanup();
- };
-
- friend struct cleanup;
-
+private:
class unique_fd {
- int m_fd;
+
+ private:
+ int m_fd{ -1 };
unique_fd( const unique_fd& ) = delete;
unique_fd& operator=( const unique_fd& ) = delete;
@@ -77,7 +73,7 @@ private:
other.m_fd = -1;
}
~unique_fd() {
- if ( m_fd != -1 ) ::close( m_fd );
+ if ( m_fd != -1 ) { ::close( m_fd ); }
}
explicit operator bool() const { return m_fd != -1; }
@@ -86,29 +82,36 @@ private:
m_fd = ::open( std::forward<Args>( args )... );
return *this;
}
+
+ int close() {
+ auto r = ::close( m_fd );
+ m_fd = -1;
+ return r;
+ }
+
+ public:
#define unique_fd_forward( fun ) \
template <typename... Args> \
- auto fun( Args&&... args ) const->decltype( ::fun( m_fd, std::forward<Args>( args )... ) ) { \
+ auto fun( Args&&... args ) const { \
return ::fun( m_fd, std::forward<Args>( args )... ); \
}
- unique_fd_forward( lseek ) unique_fd_forward( read ) unique_fd_forward( write ) unique_fd_forward( fcntl )
- unique_fd_forward( fsync ) unique_fd_forward( fchown ) unique_fd_forward( stat )
+ // clang-format off
+ unique_fd_forward( lseek )
+ unique_fd_forward( read )
+ unique_fd_forward( write )
+ unique_fd_forward( fcntl )
+ unique_fd_forward( fsync )
+ unique_fd_forward( fchown )
+ unique_fd_forward( stat )
+ // clang-format on
#undef unique_fd_forward
- int close() {
- auto r = ::close( m_fd );
- m_fd = -1;
- return r;
- }
};
- unique_fd fd;
- double pg_size;
- procInfo curr;
- std::string fname;
- char buf[500];
- bool valid;
-
- static ProcStats* inst;
+ unique_fd m_ufd;
+ double m_pg_size{ 0 };
+ procInfo m_curr;
+ std::string m_fname;
+ char m_buf[500];
+ bool m_valid{ false };
+ std::mutex m_mutex;
};
-
-#endif
--
GitLab