Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use specific implementation to get available host memory on MacOS #2597

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
5 changes: 2 additions & 3 deletions src/coreComponents/common/LifoStorageCommon.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include <mutex>
#include <condition_variable>
#include <camp/camp.hpp>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <algorithm>
Expand All @@ -38,7 +37,7 @@
#include "common/TimingMacros.hpp"
#include "common/FixedSizeDequeWithMutexes.hpp"
#include "common/MultiMutexesLock.hpp"

#include "common/getAvailableMemory.hpp"

namespace geos
{
Expand Down Expand Up @@ -155,7 +154,7 @@ class LifoStorageCommon
static int computeNumberOfBufferOnHost( int percent, size_t bufferSize, int maxNumberOfBuffers, int numberOfBuffersToStoreOnDevice )
{
GEOS_ERROR_IF( percent > 100, "Error, percentage of memory should be smallerer than -100, check lifoOnHost (should be greater that -100)" );
size_t free = sysconf( _SC_AVPHYS_PAGES ) * sysconf( _SC_PAGESIZE );
size_t free = getAvailableMemory();
int numberOfBuffersToStoreOnHost = std::max( 1, std::min( ( int )( 0.01 * percent * free / bufferSize ), maxNumberOfBuffers - numberOfBuffersToStoreOnDevice ) );
double freeGB = ( ( double ) free ) / ( 1024.0 * 1024.0 * 1024.0 ) / MpiWrapper::nodeCommSize();
LIFO_LOG_RANK( " LIFO : available memory on host " << freeGB << " GB" );
Expand Down
56 changes: 56 additions & 0 deletions src/coreComponents/common/getAvailableMemory.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#ifndef GET_AVAILABLE_MEMORY_HPP
#define GET_AVAILABLE_MEMORY_HPP

#include <sys/stat.h>
#if defined __MACH__
#include <sys/types.h>
#include <sys/sysctl.h>

#include <mach/host_info.h>
#include <mach/mach_host.h>
#include <mach/task_info.h>
#include <mach/task.h>
#endif

namespace geos
{
/**
* @brief Retieves current available memory on host
* @return the available memory in bytes.
*/
static inline size_t getAvailableMemory()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe indicate in the function name that this is for host/system memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea.

{
#if defined(__APPLE__) && defined(__MACH__)
int mib[6];
mib[0] = CTL_HW;
mib[1] = HW_PAGESIZE;

int pagesize;
size_t length;
length = sizeof( pagesize );
if( sysctl( mib, 2, &pagesize, &length, NULL, 0 ) < 0 )
{
fprintf( stderr, "getting page size" );
}

mach_msg_type_number_t count = HOST_VM_INFO_COUNT;

vm_statistics_data_t vmstat;
if( host_statistics( mach_host_self(), HOST_VM_INFO, ( host_info_t ) &vmstat, &count ) != KERN_SUCCESS )
{
fprintf ( stderr, "Failed to get VM statistics." );
}

return vmstat.free_count * pagesize;
#else
GEOS_ERROR_IF( percent > 100, "Error, percentage of memory should be smallerer than -100, check lifoOnHost (should be greater that -100)" );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied here by accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

return (size_t)sysconf( _SC_AVPHYS_PAGES ) *(size_t) sysconf( _SC_PAGESIZE );
#endif
}
}

// remove these definitions from mach/boolean.h that can conflict with GEOS code (eg. InputFlags::FALSE)
#undef TRUE
#undef FALSE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you instead encapsulate this implementation and all system-specific includes in a separate translation unit (.cpp)? I think LvArray/system.[h/c]pp would be a good place to expose this functionality, if you can spare the effort to open an LvArray PR.

Copy link
Contributor Author

@XL64 XL64 Jul 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change that, I tend to avoid commiting to LvArray as it is another project but another translation unit would be cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


#endif