Skip to content

Commit c39a2ab

Browse files
razdoburdinibhati
andauthored
Add MacOS validation to CI (#111)
This PR adds MacOS validation with clang in github workflow. Some changes in the codebase and tests are also made for compatibility with MacOS. --------- Co-authored-by: Dmitry Razdoburdin <> Co-authored-by: ibhati <[email protected]>
1 parent 9dff5dd commit c39a2ab

File tree

13 files changed

+184
-31
lines changed

13 files changed

+184
-31
lines changed

.github/workflows/build-macos.yaml

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# Copyright 2025 Intel Corporation
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
name: MacOS Build and Test
16+
17+
on:
18+
push:
19+
branches:
20+
- main
21+
pull_request:
22+
23+
permissions:
24+
contents: read
25+
26+
# This allows a subsequently queued workflow run to interrupt previous runs
27+
concurrency:
28+
group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}'
29+
cancel-in-progress: true
30+
31+
jobs:
32+
build:
33+
name: ${{ matrix.cxx }}, ${{ matrix.build_type }}
34+
runs-on: macos-latest
35+
strategy:
36+
matrix:
37+
build_type: [RelWithDebugInfo]
38+
cxx: [clang++-15]
39+
include:
40+
- cxx: clang++-15
41+
package: llvm@15
42+
cc_name: clang
43+
cxx_name: clang++
44+
needs_prefix: true
45+
46+
steps:
47+
- uses: actions/checkout@v4
48+
49+
- name: Install Compiler
50+
run: |
51+
echo "Installing ${{ matrix.package }}..."
52+
brew install ${{ matrix.package }}
53+
54+
- name: Configure build
55+
working-directory: ${{ runner.temp }}
56+
env:
57+
TEMP_WORKSPACE: ${{ runner.temp }}
58+
run: |
59+
if [[ "${{ matrix.needs_prefix }}" == "true" ]]; then
60+
# For non-default packages like llvm@15, get the install prefix
61+
COMPILER_PREFIX=$(brew --prefix ${{ matrix.package }})
62+
export CC="${COMPILER_PREFIX}/bin/${{ matrix.cc_name }}"
63+
export CXX="${COMPILER_PREFIX}/bin/${{ matrix.cxx_name }}"
64+
else
65+
# For versioned GCC installs, the name is usually directly available
66+
export CC="${{ matrix.cc_name }}"
67+
export CXX="${{ matrix.cxx_name }}"
68+
fi
69+
70+
cmake -B${TEMP_WORKSPACE}/build -S${GITHUB_WORKSPACE} \
71+
-DCMAKE_BUILD_TYPE=${{ matrix.build_type }} \
72+
-DSVS_BUILD_BINARIES=YES \
73+
-DSVS_BUILD_TESTS=YES \
74+
-DSVS_BUILD_EXAMPLES=YES \
75+
-DSVS_EXPERIMENTAL_LEANVEC=YES
76+
77+
- name: Build Tests and Utilities
78+
working-directory: ${{ runner.temp }}/build
79+
run: make -j$(sysctl -n hw.ncpu)
80+
81+
- name: Run tests
82+
env:
83+
CTEST_OUTPUT_ON_FAILURE: 1
84+
working-directory: ${{ runner.temp }}/build/tests
85+
run: ctest -C ${{ matrix.build_type }}

benchmark/include/svs-benchmark/benchmark.h

+13-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,19 @@ class SaveDirectoryChecker {
115115
~SaveDirectoryChecker() = default;
116116

117117
private:
118-
std::unordered_set<std::filesystem::path> directories_{};
118+
#if defined(__APPLE__)
119+
// Custom hash for macOS
120+
struct PathHash {
121+
std::size_t operator()(const std::filesystem::path& p) const {
122+
return std::hash<std::string>{}(p.string());
123+
}
124+
};
125+
using PathSet = std::unordered_set<std::filesystem::path, PathHash>;
126+
#else
127+
using PathSet = std::unordered_set<std::filesystem::path>;
128+
#endif // __APPLE__
129+
130+
PathSet directories_{};
119131
};
120132

121133
// Place-holder to indicate no extra arguments need forwarding to inner calls.

include/svs/core/allocator.h

+41-16
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,16 @@
4141
#include <array>
4242
#include <fcntl.h>
4343
#include <filesystem>
44+
45+
// <linux/mman.h> provides some linux-specific flags like
46+
// MAP_POPULATE, MAP_NORESERVE, MAP_HUGETLB.
47+
// They can be used only with linux, and are not availiable
48+
// for MacOS
49+
50+
#if defined(__linux__)
4451
#include <linux/mman.h>
52+
#endif // __linux__
53+
4554
#include <memory>
4655
#include <optional>
4756
#include <string>
@@ -78,11 +87,19 @@ struct HugepageX86Parameters {
7887
};
7988

8089
// Hugepage Allocation will happen in the order given below.
90+
//
91+
// MAP_HUGETLB, MAP_HUGE_1GB, MAP_HUGE_2MB are linux-specific
92+
//
93+
#if defined(__linux__)
8194
static constexpr std::array<HugepageX86Parameters, 3> hugepage_x86_options{
8295
HugepageX86Parameters{1 << 30, MAP_HUGETLB | MAP_HUGE_1GB},
8396
HugepageX86Parameters{1 << 21, MAP_HUGETLB | MAP_HUGE_2MB},
8497
HugepageX86Parameters{1 << 12, 0},
8598
};
99+
#else
100+
static constexpr std::array<HugepageX86Parameters, 1> hugepage_x86_options{
101+
HugepageX86Parameters{1 << 12, 0}};
102+
#endif // __linux__
86103

87104
namespace detail {
88105

@@ -104,14 +121,21 @@ struct HugepageAllocation {
104121
auto pagesize = params.pagesize;
105122
auto flags = params.mmap_flags;
106123
sz = lib::round_up_to_multiple_of(bytes, pagesize);
107-
ptr = mmap(
108-
nullptr,
109-
sz,
110-
PROT_READ | PROT_WRITE,
111-
MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE | flags,
112-
-1,
113-
0
114-
);
124+
125+
int mmap_flags = MAP_PRIVATE;
126+
#if defined(MAP_ANONYMOUS)
127+
mmap_flags |= MAP_ANONYMOUS;
128+
#elif defined(MAP_ANON)
129+
mmap_flags |= MAP_ANON;
130+
#else
131+
#error "System does not support anonymous mmap (no MAP_ANONYMOUS or MAP_ANON)"
132+
#endif
133+
// Add Linux-specific flags
134+
#ifdef __linux__
135+
mmap_flags |= MAP_POPULATE;
136+
#endif // __linux__
137+
138+
ptr = mmap(nullptr, sz, PROT_READ | PROT_WRITE, mmap_flags | flags, -1, 0);
115139

116140
if (ptr != MAP_FAILED) {
117141
break;
@@ -489,15 +513,16 @@ class MemoryMapper {
489513
}
490514
}
491515
lseek(fd, 0, SEEK_SET);
516+
517+
int mmap_flags = MAP_SHARED; // Accessible from all processes
518+
// Add Linux-specific flags
519+
#ifdef __linux__
520+
mmap_flags |= MAP_NORESERVE; // Don't reserve space in DRAM for this until used
521+
mmap_flags |= MAP_POPULATE; // Populate page table entries in the DRAM
522+
#endif // __linux__
523+
492524
void* base = ::mmap(
493-
nullptr,
494-
bytes.value(),
495-
mmap_permissions(permission_),
496-
MAP_NORESERVE // Don't reserve space in DRAM for this until used
497-
| MAP_SHARED // Accessible from all processes
498-
| MAP_POPULATE, // Populate page table entries in the DRAM
499-
fd,
500-
0
525+
nullptr, bytes.value(), mmap_permissions(permission_), mmap_flags, fd, 0
501526
);
502527
close(fd);
503528

include/svs/lib/datatype.h

+7
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,13 @@ template<> inline constexpr DataType datatype_v<uint16_t> = DataType::uint16;
276276
template<> inline constexpr DataType datatype_v<uint32_t> = DataType::uint32;
277277
template<> inline constexpr DataType datatype_v<uint64_t> = DataType::uint64;
278278

279+
#if defined(__APPLE__)
280+
template<> inline constexpr DataType datatype_v<size_t> =
281+
sizeof(size_t) == 8 ? DataType::uint64 :
282+
sizeof(size_t) == 4 ? DataType::uint32 :
283+
DataType::undef;
284+
#endif // __APPLE__
285+
279286
template<> inline constexpr DataType datatype_v<int8_t> = DataType::int8;
280287
template<> inline constexpr DataType datatype_v<int16_t> = DataType::int16;
281288
template<> inline constexpr DataType datatype_v<int32_t> = DataType::int32;

include/svs/lib/memory.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ template <typename T> struct Allocator {
6161
}
6262

6363
constexpr void deallocate(value_type* ptr, size_t count) noexcept {
64-
::operator delete(static_cast<void*>(ptr), sizeof(T) * count, std::align_val_t(alignof(T)));
64+
::operator delete(
65+
static_cast<void*>(ptr), sizeof(T) * count, std::align_val_t(alignof(T))
66+
);
6567
}
6668

6769
// Intercept zero-argument construction to do default initialization.

include/svs/lib/spinlock.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ namespace svs {
2222

2323
namespace detail {
2424
inline void pause() {
25-
#if defined(__i386__) || defined(__x86_64__)
25+
#if defined(__i386__) || defined(__x86_64__)
2626
__builtin_ia32_pause();
27-
#else // __aarch64__
28-
asm volatile ("yield" ::: "memory");
29-
#endif
27+
#else // __aarch64__
28+
asm volatile("yield" ::: "memory");
29+
#endif
3030
}
3131
} // namespace detail
3232

include/svs/lib/threads/thunks.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ template <typename F, typename I> struct Thunk<F, DynamicPartition<I>> {
104104
return;
105105
}
106106

107-
auto stop = std::min(grainsize * (i + 1), iterator_size);
107+
auto stop =
108+
std::min(grainsize * (i + 1), static_cast<uint64_t>(iterator_size));
108109
auto this_range =
109110
IteratorPair{std::begin(space) + start, std::begin(space) + stop};
110111
f(this_range, tid);

tests/integration/inverted/build.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ void run_test(const Queries& queries, ThreadPoolProto threadpool_proto) {
6464
auto strategy = Strategy();
6565

6666
// Distance between the obtained results and reference ressults.
67-
const double epsilon = 0.005;
67+
const double epsilon = 0.01;
68+
6869
constexpr svs::DistanceType distance_type = svs::distance_type_v<decltype(distance)>;
6970
auto expected_results = test_dataset::inverted::expected_build_results(
7071
distance_type, svsbenchmark::Uncompressed(svs::DataType::float32)

tests/svs/core/allocator.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ static_assert(std::is_same_v<typename Traits::pointer, float*>);
4242
static_assert(std::is_same_v<typename Traits::const_pointer, const float*>);
4343
static_assert(std::is_same_v<typename Traits::void_pointer, void*>);
4444
static_assert(std::is_same_v<typename Traits::const_void_pointer, const void*>);
45+
#if defined(__APPLE__)
46+
static_assert(std::is_same_v<typename Traits::difference_type, long>);
47+
#else
4548
static_assert(std::is_same_v<typename Traits::difference_type, int64_t>);
49+
#endif // __APPLE__
4650
static_assert(std::is_same_v<typename Traits::size_type, size_t>);
4751
static_assert(std::is_same_v<
4852
typename Traits::propagate_on_container_copy_assignment,

tests/svs/core/distances/simd_utils.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,4 @@ CATCH_TEST_CASE("Masks", "[distance]") {
6464
CATCH_REQUIRE(svs::create_mask<16>(svs::lib::MaybeStatic<16>()) == 0xFFFF);
6565
}
6666

67-
#endif // defined(__i386__) || defined(__x86_64__)
67+
#endif // defined(__i386__) || defined(__x86_64__)

tests/svs/index/inverted/clustering.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,8 @@ CATCH_TEST_CASE("Clustering with Logger", "[logging]") {
412412
auto clustering_parameters = svs::index::inverted::ClusteringParameters()
413413
.percent_centroids(svs::lib::Percent(0.1))
414414
.epsilon(0.05)
415-
.max_replicas(8)
416-
.max_cluster_size(200);
415+
.max_replicas(12)
416+
.max_cluster_size(300);
417417
auto centroids = svs::index::inverted::randomly_select_centroids(
418418
data.size(),
419419
svs::lib::narrow_cast<size_t>(

tests/svs/lib/threads/threadpool.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ CATCH_TEST_CASE("Thread Pool", "[core][threads][threadpool]") {
117117
[&](const auto& range, uint64_t tid) {
118118
std::lock_guard lock{mutex};
119119
seen_threads.push_back(tid);
120-
ranges.push_back(threads::UnitRange<size_t>{
120+
ranges.push_back(threads::UnitRange<uint64_t>{
121121
*(range.begin()), *(range.end())});
122122
}
123123
);

tests/svs/lib/timing.cpp

+19-3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,22 @@ void stress(svs::lib::Timer& timer, size_t max_depth = 3, size_t max_flat = 3) {
4545
}
4646
}
4747
}
48+
49+
//
50+
// We use a busy_sleep instead of std::this_thread::sleep_for.
51+
// std::this_thread::sleep_for guarantees that the sleep will be
52+
// not shorter than the required time.
53+
// For some platforms (like MacOS), a sleep can be much longer,
54+
// so if we need the sleep time to be close to the required,
55+
// busy sleep is a good choice.
56+
57+
void busy_sleep(std::chrono::nanoseconds duration) {
58+
auto start = std::chrono::steady_clock::now();
59+
while (std::chrono::steady_clock::now() - start < duration) {
60+
// spin
61+
}
62+
}
63+
4864
} // namespace
4965

5066
CATCH_TEST_CASE("Timing", "[lib][timing]") {
@@ -66,16 +82,16 @@ CATCH_TEST_CASE("Timing", "[lib][timing]") {
6682
auto timer = svs::lib::Timer();
6783
{
6884
auto x = timer.push_back("a");
69-
std::this_thread::sleep_for(std::chrono::milliseconds(10));
85+
busy_sleep(std::chrono::milliseconds(10));
7086
}
7187
{
7288
auto x = timer.push_back("b");
73-
std::this_thread::sleep_for(std::chrono::milliseconds(10));
89+
busy_sleep(std::chrono::milliseconds(10));
7490
}
7591
{
7692
auto x = timer.push_back("b");
7793
auto y = timer.push_back("c");
78-
std::this_thread::sleep_for(std::chrono::milliseconds(10));
94+
busy_sleep(std::chrono::milliseconds(10));
7995
}
8096

8197
// Number of elapsed time should be pretty close to the sleep time.

0 commit comments

Comments
 (0)