Starting tidy

cpp/CMakeLists.txt

@@ -26,6 +26,7 @@ option(ARCTICDB_USING_ADDRESS_SANITIZER "Enable address sanitizer." OFF)
 option(ARCTICDB_USING_THREAD_SANITIZER "Enable thread sanitizer." OFF)
 option(ARCTICDB_USING_UB_SANITIZER "Enable undefined behavior sanitizer." OFF)
 option(ARCTICDB_LOG_PERFORMANCE "Whether to log performance timings." OFF)
+option(ARCTICDB_COUNT_ALLOCATION "Override new and delete to count allocations." OFF)
 
 set(ARCTICDB_SANITIZER_FLAGS)
 if(${ARCTICDB_USING_ADDRESS_SANITIZER})
@@ -47,6 +48,17 @@ if(${ARCTICDB_LOG_PERFORMANCE})
     add_compile_definitions(ARCTICDB_LOG_PERFORMANCE)
 endif()
 
+if(${ARCTICDB_COUNT_ALLOCATIONS})
+    add_compile_definitions(ARCTICDB_COUNT_ALLOCATIONS)
+    include(FetchContent)
+    FetchContent_Declare(
+            cpptrace
+            GIT_REPOSITORY https://github.com/jeremy-rifkin/cpptrace.git
+            GIT_TAG        v0.7.3 # <HASH or TAG>
+    )
+    FetchContent_MakeAvailable(cpptrace)
+endif()
+
 if(ARCTICDB_SANITIZER_FLAGS)
     list(APPEND ARCTICDB_SANITIZER_FLAGS "-g;-fno-omit-frame-pointer;-fno-optimize-sibling-calls;")
     if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")

cpp/arcticdb/CMakeLists.txt

@@ -266,6 +266,7 @@ set(arcticdb_srcs
         processing/query_planner.hpp
         processing/sorted_aggregation.hpp
         processing/unsorted_aggregation.hpp
+        storage/async_storage.hpp
         storage/constants.hpp
         storage/common.hpp
         storage/config_resolvers.hpp
@@ -326,6 +327,7 @@ set(arcticdb_srcs
         stream/stream_writer.hpp
         toolbox/library_tool.hpp
         util/allocator.hpp
+        util/allocation_tracing.hpp
         util/bitset.hpp
         util/buffer.hpp
         util/buffer_pool.hpp
@@ -488,6 +490,7 @@ set(arcticdb_srcs
         stream/protobuf_mappings.cpp
         toolbox/library_tool.cpp
         util/allocator.cpp
+        util/allocation_tracing.cpp
         util/buffer_pool.cpp
         util/configs_map.cpp
         util/decimal.cpp
@@ -511,7 +514,7 @@ set(arcticdb_srcs
         version/symbol_list.cpp
         version/version_map_batch_methods.cpp
         storage/s3/ec2_utils.cpp
-        util/buffer_holder.cpp storage/async_storage.hpp util/allocation_tracing.hpp util/allocation_tracing.cpp storage/s3/http/http_client.cpp storage/s3/http/http_client.hpp storage/s3/http/http_client_factory.hpp storage/s3/http/http_client_factory.cpp storage/s3/http/http_request.hpp storage/s3/http/http_request.cpp storage/s3/http/http_response.hpp storage/s3/http/http_response.cpp)
+        util/buffer_holder.cpp)
 
 add_library(arcticdb_core_object OBJECT ${arcticdb_srcs})
 
@@ -662,12 +665,17 @@ set (arcticdb_core_libraries
         ${Zstd_LIBRARY}
         ${ARCTICDB_MONGO_LIBS}
         spdlog::spdlog
-        cpptrace::cpptrace
         Folly::folly # Transitively includes: double-conversion, gflags, glog, libevent, libssl, libcrypto, libiberty, libsodium
         Azure::azure-identity
         Azure::azure-storage-blobs
 )
 
+if(${ARCTICDB_COUNT_ALLOCATIONS})
+    list(APPEND arcticdb_core_libraries
+            cpptrace::cpptrace
+    )
+endif()
+
 if(${ARCTICDB_PYTHON_EXPLICIT_LINK})
     # Even though python is transitive dependency MSVS builds fail if we don't link against python explicitly
     # TODO: Figure out why

cpp/arcticdb/async/test/test_async.cpp

@@ -23,9 +23,9 @@
 
 #include <arcticdb/storage/s3/s3_api.hpp>
 #include <arcticdb/storage/s3/s3_storage.hpp>
-#include "arcticdb/storage/s3/mock/s3_mock_client.hpp"
+#include <arcticdb/storage/mock/s3_mock_client.hpp>
 #include <arcticdb/storage/s3/detail-inl.hpp>
-#include "arcticdb/storage/mock/storage_mock_client.hpp"
+#include <arcticdb/storage/mock/storage_mock_client.hpp>
 #include <aws/core/Aws.h>
 
 using namespace arcticdb;

cpp/arcticdb/storage/test/common.hpp

@@ -14,7 +14,7 @@
 
 namespace arcticdb::storage {
 
-inline VariantKey get_test_key(std::string name, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
+inline VariantKey get_test_key(const std::string& name, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
     auto builder = arcticdb::atom_key_builder();
     return builder.build(name, key_type);
 }
@@ -25,22 +25,22 @@ inline Segment get_test_segment() {
     return encode_dispatch(std::move(segment_in_memory), codec_opts, arcticdb::EncodingVersion::V2);
 }
 
-inline void write_in_store(Storage &store, std::string symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
+inline void write_in_store(Storage &store, const std::string& symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
     auto variant_key = get_test_key(symbol, key_type);
     store.write(KeySegmentPair(std::move(variant_key), get_test_segment()));
 }
 
-inline void update_in_store(Storage &store, std::string symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
+inline void update_in_store(Storage &store, const std::string& symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
     auto variant_key = get_test_key(symbol, key_type);
     store.update(KeySegmentPair(std::move(variant_key), get_test_segment()), arcticdb::storage::UpdateOpts{});
 }
 
-inline bool exists_in_store(Storage &store, std::string symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
+inline bool exists_in_store(Storage &store, const std::string& symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
     auto variant_key = get_test_key(symbol, key_type);
     return store.key_exists(variant_key);
 }
 
-inline std::string read_in_store(Storage &store, std::string symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
+inline std::string read_in_store(Storage &store, const std::string& symbol, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {
     auto variant_key = get_test_key(symbol, key_type);
     auto opts = ReadKeyOpts{};
     auto result = store.read(std::move(variant_key), opts);
@@ -53,7 +53,7 @@ inline void remove_in_store(Storage &store, const std::vector<std::string>& symb
         to_remove.emplace_back(get_test_key(symbol, key_type));
     }
     auto opts = RemoveOpts();
-    store.remove(Composite(std::move(to_remove)), opts);
+    store.remove(std::span(to_remove), opts);
 }
 
 inline std::set<std::string> list_in_store(Storage &store, entity::KeyType key_type = entity::KeyType::TABLE_DATA) {

cpp/arcticdb/storage/test/test_s3_storage.cpp

@@ -10,7 +10,7 @@
 #include <arcticdb/storage/storage.hpp>
 #include <arcticdb/storage/s3/s3_api.hpp>
 #include <arcticdb/storage/s3/s3_storage.hpp>
-#include "arcticdb/storage/s3/mock/s3_mock_client.hpp"
+#include <arcticdb/storage/mock/s3_mock_client.hpp>
 #include <arcticdb/storage/s3/detail-inl.hpp>
 #include <arcticdb/entity/protobufs.hpp>
 #include <arcticdb/entity/variant_key.hpp>

cpp/arcticdb/storage/test/test_storage_exceptions.cpp

@@ -13,7 +13,7 @@
 #include "arcticdb/storage/mock/lmdb_mock_client.hpp"
 #include <arcticdb/storage/memory/memory_storage.hpp>
 #include <arcticdb/storage/s3/s3_storage.hpp>
-#include "arcticdb/storage/s3/mock/s3_mock_client.hpp"
+#include "arcticdb/storage/mock/s3_mock_client.hpp"
 #include <arcticdb/storage/azure/azure_storage.hpp>
 #include "arcticdb/storage/mock/azure_mock_client.hpp"
 #include <arcticdb/storage/mongo/mongo_storage.hpp>

cpp/arcticdb/util/allocation_tracing.cpp

@@ -1,5 +1,6 @@
 #include <arcticdb/util/allocation_tracing.hpp>
 
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
 
 namespace arcticdb {
 
@@ -39,7 +40,7 @@ std::shared_ptr<AllocationTracker> AllocationTracker::instance_;
 std::once_flag AllocationTracker::init_flag_;
 
 } // namespace arcticdb
-/*
+
 void* operator new(std::size_t sz){
     void* ptr = std::malloc(sz);
     if(arcticdb::AllocationTracker::started())
@@ -54,4 +55,5 @@ void operator delete(void* ptr) noexcept{
 void operator delete(void* ptr, std::size_t) noexcept{
     std::free(ptr);
 }
-*/
+
+#endif

cpp/arcticdb/util/allocation_tracing.hpp

@@ -14,10 +14,10 @@
 #include <ankerl/unordered_dense.h>
 #include <iostream>
 
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
 
 namespace arcticdb {
 
-
 class AllocationTracker {
     static std::shared_ptr<AllocationTracker> instance_;
     static std::once_flag init_flag_;
@@ -58,9 +58,13 @@ class AllocationTracker {
 };
 
 }
-/*
+
+
+
 void* operator new(std::size_t sz);
 
 void operator delete(void* ptr) noexcept;
 
-void operator delete(void* ptr, std::size_t) noexcept;*/
+void operator delete(void* ptr, std::size_t) noexcept;
+
+#endif

cpp/arcticdb/util/global_lifetimes.cpp

@@ -24,7 +24,9 @@
 namespace arcticdb {
 
 ModuleData::~ModuleData() {
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
     AllocationTracker::destroy_instance();
+#endif
     BufferPool::destroy_instance();
     TracingData::destroy_instance();
     Allocator::destroy_instance();

cpp/arcticdb/util/trace.cpp

@@ -10,15 +10,11 @@
 
 #ifndef  _WIN32
 #include <cxxabi.h>
-#include <execinfo.h>
 #endif
 
-
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
 #include <cpptrace/cpptrace.hpp>
-
-std::string get_trace() {
-    return cpptrace::generate_trace(5, 10).to_string();
-}
+#endif
 
 namespace arcticdb {
 
@@ -33,19 +29,24 @@ std::string get_type_name(const std::type_info& ti) {
 #endif
 }
 
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
+
+std::string get_trace() {
+    return cpptrace::generate_trace(5, 10).to_string();
+}
+
 std::string_view removePrefix(std::string_view input, std::string_view prefix) {
     auto pos = input.rfind(prefix);
     if (pos != std::string_view::npos) {
         return {input.data() + prefix.size(), input.size() - prefix.size()};
     }
-    return input; // Return the original string if prefix is not found
+    return input;
 }
 
 std::string unwind_stack(int) {
     return get_trace();
 }
 
-/*
 std::string unwind_stack(int max_depth) {
     void *buffer[max_depth];
     int num_frames = backtrace(buffer, max_depth);
@@ -60,5 +61,7 @@ std::string unwind_stack(int max_depth) {
     free(symbols);
     return oss.str();
 }
-*/
+
+#endif
+
 } // namespace arcticdb

cpp/arcticdb/util/trace.hpp

@@ -14,6 +14,7 @@ namespace arcticdb {
 
 std::string get_type_name(const std::type_info& type_info);
 
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
 std::string unwind_stack(int max_depth);
-
+#endif
 }  // namespace arcticdb

cpp/arcticdb/version/local_versioned_engine.cpp

@@ -55,8 +55,10 @@ void LocalVersionedEngine::initialize(const std::shared_ptr<storage::Library>& l
         async::TaskScheduler::reattach_instance();
     }
     (void)async::TaskScheduler::instance();
+#ifdef ARCTICDB_COUNT_ALLOCATIONS
     (void)AllocationTracker::instance();
     AllocationTracker::start();
+#endif
 }
 
 template LocalVersionedEngine::LocalVersionedEngine(const std::shared_ptr<storage::Library>& library, const util::SysClock&);