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

fix: Check for existence of_ SC_AVPHYS_PAGES to fix MacOS compilation. #3207

Merged
merged 9 commits into from
Jul 8, 2024
10 changes: 10 additions & 0 deletions .github/workflows/ci_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,16 @@ jobs:
GCP_BUCKET: geosx/ubuntu22.04-gcc11
RUNS_ON: Runner_4core_16GB

# mac_builds:
# needs:
# - is_not_draft_pull_request
# runs-on: macos-14-xlarge
# steps:
# - run: sysctl -n hw.physicalcpu
# - run: sysctl -h hw.memsize
# - run: sysctl -n machdep.cpu.brand_string


# If the 'ci: run CUDA builds' PR label is found, the cuda jobs run immediately along side linux jobs.
# Note: CUDA jobs should only be run if PR is ready to merge.
cuda_builds:
Expand Down
5 changes: 5 additions & 0 deletions src/coreComponents/common/MemoryInfos.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@ MemoryInfos::MemoryInfos( umpire::MemoryResourceTraits::resource_type resourceTy
case umpire::MemoryResourceTraits::resource_type::pinned:
#if defined( _SC_PHYS_PAGES ) && defined( _SC_PAGESIZE )
m_totalMemory = sysconf( _SC_PHYS_PAGES ) * sysconf( _SC_PAGESIZE );
#if defined(_SC_AVPHYS_PAGES)
m_availableMemory = sysconf( _SC_AVPHYS_PAGES ) * sysconf( _SC_PAGESIZE );
#else
Copy link
Contributor

@MelReyCG MelReyCG Jul 3, 2024

Choose a reason for hiding this comment

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

Would something like this be a way to find the available memory on MaxOS? Unfortunately, I we don't have any MacOS to test that here.

Suggested change
#else
#elif /* is compiling for MacOS */
{
uint64_t mem;
size_t len = sizeof( mem );
sysctlbyname( "hw.memsize", &mem, &len, nullptr, 0 );
m_availableMemory= mem / sysconf(_SC_PAGE_SIZE);
}

Should we regroup the system-dependant component like this in a namespace ?

GEOS_WARNING( "Unknown device avaialable memory size getter for this system." );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
GEOS_WARNING( "Unknown device avaialable memory size getter for this system." );
GEOS_WARNING( "Unknown device available memory size getter for this system." );

m_availableMemory = 0;
#endif
#else
GEOS_WARNING( "Unknown device physical memory size getter for this compiler." );
m_physicalMemoryHandled = 0;
#endif
Expand Down
Loading