Merge pull request #2900 from rnlahaye/magic_alloc

Use system allocator for containers on macOS (bypassing Unity's allocator)

Makefile

@@ -87,6 +87,7 @@ SHARED_ARGS   := \
 	-Wall -Wpedantic                                              \
 	-Wno-char-subscripts                                          \
 	-Wno-gnu-anonymous-struct                                     \
+	-Wno-c99-extensions                                           \
 	-Wno-gnu-zero-variadic-macro-arguments                        \
 	-Wno-nested-anon-types                                        \
 	-Wno-unknown-pragmas                                          \
@@ -106,7 +107,11 @@ ifeq ($(UNAME_S),Linux)
     SHAREDFLAG := -shared
 endif
 ifeq ($(UNAME_S),Darwin)
-    INCLUDES += -I$(DEP_DIR)compatibility/filesystem -I$(DEP_DIR)compatibility/optional -I$(DEP_DIR)Optional
+    INCLUDES += \
+			-I$(DEP_DIR)compatibility/filesystem \
+			-I$(DEP_DIR)compatibility/optional \
+			-I$(DEP_DIR)Optional \
+			-include "base/macos_allocator_replacement.hpp"
     SHARED_ARGS += -mmacosx-version-min=10.12 -arch x86_64 -D_LIBCPP_STD_VER=16
     SHAREDFLAG := -dynamiclib
 endif

base/macos_allocator_replacement.hpp

@@ -0,0 +1,68 @@
+// This file contains aliases of some STL containers (such as `std::vector`) in
+// the `principia::std` namespace with their default allocators overridden to
+// base new one defined here. The result of this is that if you write
+// `std::vector foo` inside the principia namespace, it will resolve to a vector
+// with the allocator defined here.
+//
+// The intended purpose of this file is to be used on macOS builds. This is
+// necessary because Unity provides overrides of `new` and `delete` which have
+// terrible performance on macOS. It should be included via compiler argument.
+// Attempting to include it on non-macOS builds will result in an error.
+
+#pragma once
+
+#include <deque>
+#include <list>
+#include <map>
+#include <set>
+#include <utility>
+#include <vector>
+
+#include "base/macros.hpp"
+#include "base/malloc_allocator.hpp"
+
+#if !OS_MACOSX
+#error Only include this file for macOS
+#endif
+
+namespace principia {
+namespace std {
+
+using namespace ::std;
+
+template<typename T>
+using allocator = ::principia::base::MallocAllocator<T>;
+
+template<typename T, typename Allocator = allocator<T>>
+using vector = ::std::vector<T, Allocator>;
+
+template<typename T, typename Allocator = allocator<T>>
+using deque = ::std::deque<T, Allocator>;
+
+template<typename T, typename Allocator = allocator<T>>
+using list = ::std::list<T, Allocator>;
+
+template<typename Key,
+         typename Compare = std::less<Key>,
+         typename Allocator = allocator<Key>>
+using set = ::std::set<Key, Compare, Allocator>;
+
+template<typename Key,
+         typename T,
+         typename Compare = std::less<Key>,
+         typename Allocator = allocator<std::pair<const Key, T>>>
+using map = ::std::map<Key, T, Compare, Allocator>;
+
+template<typename Key,
+         typename Compare = std::less<Key>,
+         typename Allocator = allocator<Key>>
+using multiset = ::std::multiset<Key, Compare, Allocator>;
+
+template<typename Key,
+         typename T,
+         typename Compare = std::less<Key>,
+         typename Allocator = allocator<std::pair<const Key, T>>>
+using multimap = ::std::multimap<Key, T, Compare, Allocator>;
+
+}  // namespace std
+}  // namespace principia

base/macos_allocator_replacement_test.cpp

@@ -0,0 +1,48 @@
+
+// "macos_allocator_replacement.hpp" should be automagically included.
+
+#include <deque>
+#include <list>
+#include <map>
+#include <set>
+#include <type_traits>
+#include <utility>
+#include <vector>
+
+#include "base/macros.hpp"
+#include "base/malloc_allocator.hpp"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#ifdef OS_MACOSX
+
+namespace principia {
+namespace magic {
+
+using base::MallocAllocator;
+
+// AllocatorIs<container, alloc>() returns true iff container's allocator is
+// alloc.
+template<typename T, typename Allocator>
+constexpr bool AllocatorIs() {
+  return std::is_same<typename T::allocator_type, Allocator>::value;
+}
+
+// Test that the default allocators for various classes are overridden.
+TEST(PrincipiaMallocAllocatorTest, DefaultAllocators) {
+  // STL
+  EXPECT_TRUE((AllocatorIs<std::vector<int>, MallocAllocator<int>>()));
+  EXPECT_TRUE((AllocatorIs<std::deque<int>, MallocAllocator<int>>()));
+  EXPECT_TRUE((AllocatorIs<std::list<int>, MallocAllocator<int>>()));
+  EXPECT_TRUE((AllocatorIs<std::set<int>, MallocAllocator<int>>()));
+  EXPECT_TRUE((AllocatorIs<std::map<int, int>,
+                           MallocAllocator<std::pair<const int, int>>>()));
+  EXPECT_TRUE((AllocatorIs<std::multiset<int>, MallocAllocator<int>>()));
+  EXPECT_TRUE((AllocatorIs<std::multimap<int, int>,
+                           MallocAllocator<std::pair<const int, int>>>()));
+}
+
+}  // namespace magic
+}  // namespace principia
+
+#endif

base/malloc_allocator.hpp

@@ -0,0 +1,48 @@
+
+#pragma once
+
+#include <cstddef>
+#include <cstdlib>
+
+namespace principia {
+namespace base {
+
+// An allocator (for use with containers such as `std::vector`) that uses malloc
+// and free for memory management instead of global new and delete. The purpose
+// of this allocator is to enable use of the system allocator even when global
+// new and delete have been overridden.
+template<typename T>
+class MallocAllocator {
+ public:
+  using value_type = T;
+  using pointer = T*;
+  using const_pointer = T const*;
+  using reference = T&;
+  using const_reference = T const&;
+  using size_type = std::size_t;
+  using difference_type = std::ptrdiff_t;
+
+  T* allocate(size_t n) {
+    return static_cast<T*>(calloc(n, sizeof(T)));
+  }
+
+  void deallocate(T* p, size_t n) {
+    free(p);
+  }
+};
+
+// MallocAllocators are equal regardless of type.
+template<typename T1, typename T2>
+constexpr bool operator==(const MallocAllocator<T1>&,
+                          const MallocAllocator<T2>&) {
+  return true;
+}
+
+template<typename T1, typename T2>
+constexpr bool operator!=(const MallocAllocator<T1>&,
+                          const MallocAllocator<T2>&) {
+  return false;
+}
+
+}  // namespace base
+}  // namespace principia

base/malloc_allocator_test.cpp

@@ -0,0 +1,42 @@
+
+#include "base/malloc_allocator.hpp"
+
+#include <cstdlib>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace principia {
+namespace base {
+
+using ::testing::IsNull;
+using ::testing::Not;
+
+TEST(MallocAllocatorTest, Allocate) {
+  int* p;
+  p = MallocAllocator<int>().allocate(1);
+  EXPECT_THAT(p, Not(IsNull()));
+  *p = 123;
+  free(p);
+
+  p = MallocAllocator<int>().allocate(10);
+  EXPECT_THAT(p, Not(IsNull()));
+  p[9] = 123;
+  free(p);
+}
+
+TEST(MallocAllocatorTest, Deallocate) {
+  int* p = static_cast<int*>(malloc(sizeof(int)));
+  MallocAllocator<int>().deallocate(p, 1);
+}
+
+TEST(MallocAllocatorTest, RoundTrip) {
+  int* p;
+  p = MallocAllocator<int>().allocate(1);
+  EXPECT_THAT(p, Not(IsNull()));
+  *p = 123;
+  MallocAllocator<int>().deallocate(p, 1);
+}
+
+}  // namespace base
+}  // namespace principia

mathematica/error_analysis_test.cpp

@@ -62,7 +62,7 @@ TEST_F(ErrorAnalysisTest,
 
 TEST_F(ErrorAnalysisTest, DISABLED_SECULAR_LocalErrorAnalysis) {
   google::LogToStderr();
-  std::vector<std::string> argv = ::testing::internal::GetArgvs();
+  ::std::vector<std::string> argv = ::testing::internal::GetArgvs();
   std::map<std::string, std::optional<std::string>> flags;
   for (int i = 2; i < argv.size(); ++i) {
     std::string const flag(argv[i]);

physics/discrete_trajectory.hpp

@@ -273,7 +273,7 @@ class DiscreteTrajectory : public Forkable<DiscreteTrajectory<Frame>,
 
   // For using the private constructor in maps.
   template<typename, typename>
-  friend struct std::pair;
+  friend struct ::std::pair;
 };
 
 }  // namespace internal_discrete_trajectory