Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ jobs:
-DFETCH_DEPENDENCIES_WITH_CMAKE=MISSING \
-DSPARROW_BUILD_SHARED=${{matrix.shared}} \
-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=mold" \
-DCMAKE_SHARED_LINKER_FLAGS="-fuse-ld=mold"
-DCMAKE_SHARED_LINKER_FLAGS="-fuse-ld=mold" \
-DTRACK_COPIES=ON

- name: Build library
working-directory: build
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/manylinux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ jobs:
-DBUILD_EXAMPLES=ON \
-DFETCH_DEPENDENCIES_WITH_CMAKE=MISSING \
-DSPARROW_BUILD_SHARED=ON \
-DCREATE_JSON_READER_TARGET=ON
-DCREATE_JSON_READER_TARGET=ON \
-DTRACK_COPIES=ON

- name: Build library
working-directory: sparrow/build
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/osx.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ jobs:
-DUSE_DATE_POLYFILL=ON \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DFETCH_DEPENDENCIES_WITH_CMAKE=MISSING
-DFETCH_DEPENDENCIES_WITH_CMAKE=MISSING \
-DTRACK_COPIES=ON

- name: Build
working-directory: build
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/qemu.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ jobs:
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache
-DCMAKE_C_COMPILER_LAUNCHER=ccache
-DSPARROW_CONTRACTS_THROW_ON_FAILURE=ON
-DTRACK_COPIES=ON
-B build

shell: alpine.sh {0}
Expand Down
11 changes: 10 additions & 1 deletion include/sparrow/arrow_interface/arrow_array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

#pragma once

#include <cstddef>
#include <cstdint>
#include <ranges>
#include <type_traits>
Expand All @@ -26,10 +25,20 @@
#include "sparrow/arrow_interface/arrow_array/private_data.hpp"
#include "sparrow/c_interface.hpp"
#include "sparrow/config/config.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/utils/repeat_container.hpp"

namespace sparrow
{
namespace copy_tracker
{
template <>
inline std::string key<ArrowArray>()
{
return "ArrowArray";
}
}

/**
* Creates an `ArrowArray`.
*
Expand Down
10 changes: 10 additions & 0 deletions include/sparrow/arrow_interface/arrow_schema.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,21 @@
#include "sparrow/arrow_interface/arrow_schema/private_data.hpp"
#include "sparrow/c_interface.hpp"
#include "sparrow/config/config.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/utils/contracts.hpp"
#include "sparrow/utils/metadata.hpp"

namespace sparrow
{
namespace copy_tracker
{
template <>
inline std::string key<ArrowSchema>()
{
return "ArrowSchema";
}
}

/**
* Creates an `ArrowSchema` owned by a `unique_ptr` and holding the provided data.
*
Expand Down
12 changes: 12 additions & 0 deletions include/sparrow/buffer/buffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <type_traits>

#include "sparrow/buffer/allocator.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/details/3rdparty/xsimd_aligned_allocator.hpp"
#include "sparrow/utils/contracts.hpp"
#include "sparrow/utils/iterator.hpp"
Expand All @@ -35,6 +36,15 @@

namespace sparrow
{
namespace copy_tracker
{
template <typename T>
std::string key_buffer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about keeping the generic name key and constrain the template, like you did for primitive_array_impl ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

{
return "buffer<" + std::string(typeid(T).name()) + ">";
}
Comment on lines 45 to 52
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

key_buffer() uses typeid(T).name() but this header doesn't include <typeinfo>. Relying on transitive includes for std::type_info/typeid is fragile and can break with different standard library implementations. Add an explicit #include <typeinfo> (or avoid typeid here).

Copilot uses AI. Check for mistakes.
}

template <typename T>
concept is_buffer_view = requires(T t) { typename T::is_buffer_view; };

Expand Down Expand Up @@ -490,6 +500,7 @@ namespace sparrow
this->create_storage(rhs.size());
get_data().p_end = copy_initialize(rhs.begin(), rhs.end(), get_data().p_begin, get_allocator());
}
copy_tracker::increase(copy_tracker::key_buffer<T>());
}

template <class T>
Expand All @@ -502,6 +513,7 @@ namespace sparrow
this->create_storage(rhs.size());
get_data().p_end = copy_initialize(rhs.begin(), rhs.end(), get_data().p_begin, get_allocator());
}
copy_tracker::increase(copy_tracker::key_buffer<T>());
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should also inscrease copies number in operator=.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

}

template <class T>
Expand Down
22 changes: 22 additions & 0 deletions include/sparrow/buffer/buffer_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,20 @@
#pragma once

#include "sparrow/buffer/buffer.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/utils/contracts.hpp"

namespace sparrow
{
namespace copy_tracker
{
template <class T>
std::string key_buffer_view()
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about keeping the generic name key and constrain the template like you did for primitive_array_impl ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

{
return "buffer_view<" + std::string(typeid(T).name()) + ">";
}
}

/*
* Non-owning view of a contiguous sequence of objects of type T.
*
Expand Down Expand Up @@ -48,6 +58,10 @@ namespace sparrow
using const_reverse_iterator = std::reverse_iterator<const_iterator>;

constexpr buffer_view() = default;
constexpr buffer_view(const buffer_view&);
constexpr buffer_view(buffer_view&&) noexcept = default;
constexpr buffer_view& operator=(const buffer_view&) = default;
constexpr buffer_view& operator=(buffer_view&&) noexcept = default;
constexpr explicit buffer_view(buffer<T>& buffer)
requires(!std::is_const_v<T>);
template <class U>
Expand Down Expand Up @@ -116,6 +130,14 @@ namespace sparrow
* buffer_view implementation *
******************************/

template <class T>
constexpr buffer_view<T>::buffer_view(const buffer_view& other)
: p_data(other.p_data)
, m_size(other.m_size)
{
copy_tracker::increase(copy_tracker::key_buffer_view<T>());
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

buffer_view is intended to be a cheap, non-owning view, but the new copy ctor calls copy_tracker::increase(copy_tracker::key_buffer_view<T>()), which constructs a std::string (allocations) and can throw. This changes the copy from trivially/noexcept copyable to potentially-throwing and adds overhead even when copy tracking is disabled (the key string is still built). Consider making this tracking compile-time gated (e.g., #ifdef SPARROW_TRACK_COPIES / if constexpr (copy_tracker::is_enabled())) and/or using a non-allocating key type (e.g., std::string_view / const char*) so copying a view stays noexcept and cheap.

Suggested change
{
copy_tracker::increase(copy_tracker::key_buffer_view<T>());
{
#if defined(SPARROW_TRACK_COPIES)
copy_tracker::increase(copy_tracker::key_buffer_view<T>());
#endif

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should also increase copies number in operator=

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

}

template <class T>
constexpr buffer_view<T>::buffer_view(buffer<T>& buffer)
requires(!std::is_const_v<T>)
Expand Down
50 changes: 50 additions & 0 deletions include/sparrow/decimal_array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "sparrow/arrow_interface/arrow_array_schema_proxy.hpp"
#include "sparrow/arrow_interface/arrow_schema.hpp"
#include "sparrow/buffer/dynamic_bitset/dynamic_bitset.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/layout/array_bitmap_base.hpp"
#include "sparrow/layout/decimal_reference.hpp"
#include "sparrow/layout/layout_utils.hpp"
Expand All @@ -41,6 +42,16 @@ namespace sparrow
template <decimal_type T>
class decimal_array;

namespace copy_tracker
{
template <typename T>
requires mpl::is_type_instance_of_v<T, decimal_array>
std::string key()
{
return "decimal_array";
}
}

/** Type alias for 32-bit decimal array. */
using decimal_32_array = decimal_array<decimal<int32_t>>;
/** Type alias for 64-bit decimal array. */
Expand Down Expand Up @@ -192,6 +203,36 @@ namespace sparrow
*/
explicit decimal_array(arrow_proxy proxy);

/**
* Copy constructor.
*
* @param rhs The decimal array to copy from.
*/
decimal_array(const decimal_array& rhs);

/**
* Copy assignment operator.
*
* @param rhs The decimal array to assign from.
* @return Reference to this array.
*/
decimal_array& operator=(const decimal_array& rhs) = default;

/**
* Move constructor.
*
* @param rhs The decimal array to move from.
*/
decimal_array(decimal_array&& rhs) noexcept = default;

/**
* Move assignment operator.
*
* @param rhs The decimal array to move assign from.
* @return Reference to this array.
*/
decimal_array& operator=(decimal_array&& rhs) noexcept = default;

/**
* Constructs a decimal array with the given arguments.
*
Expand Down Expand Up @@ -536,6 +577,15 @@ namespace sparrow
}
}

template <decimal_type T>
decimal_array<T>::decimal_array(const decimal_array& rhs)
: base_type(rhs)
, m_precision(rhs.m_precision)
, m_scale(rhs.m_scale)
{
copy_tracker::increase(copy_tracker::key<decimal_array<T>>());
}

template <decimal_type T>
template <std::ranges::input_range VALUE_RANGE, validity_bitmap_input VALIDITY_RANGE, input_metadata_container METADATA_RANGE>
requires std::convertible_to<std::ranges::range_value_t<VALUE_RANGE>, typename T::integer_type>
Expand Down
12 changes: 12 additions & 0 deletions include/sparrow/dictionary_encoded_array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "sparrow/arrow_interface/arrow_array_schema_proxy.hpp"
#include "sparrow/buffer/dynamic_bitset/dynamic_bitset.hpp"
#include "sparrow/c_interface.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/layout/array_access.hpp"
#include "sparrow/layout/array_factory.hpp"
#include "sparrow/layout/array_helper.hpp"
Expand Down Expand Up @@ -91,6 +92,16 @@ namespace sparrow
template <std::integral IT>
class dictionary_encoded_array;

namespace copy_tracker
{
template <typename T>
requires mpl::is_type_instance_of_v<T, dictionary_encoded_array>
std::string key()
{
return "dictionary_encoded_array";
}
}

namespace detail
{
template <std::integral IT>
Expand Down Expand Up @@ -591,6 +602,7 @@ namespace sparrow
, m_keys_layout(create_keys_layout(m_proxy))
, p_values_layout(create_values_layout(m_proxy))
{
copy_tracker::increase(copy_tracker::key<self_type>());
}

template <std::integral IT>
Expand Down
28 changes: 28 additions & 0 deletions include/sparrow/fixed_width_binary_array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "sparrow/arrow_interface/arrow_schema.hpp"
#include "sparrow/buffer/dynamic_bitset/dynamic_bitset.hpp"
#include "sparrow/c_interface.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/layout/array_bitmap_base.hpp"
#include "sparrow/layout/fixed_width_binary_array_utils.hpp"
#include "sparrow/layout/fixed_width_binary_reference.hpp"
Expand Down Expand Up @@ -79,6 +80,15 @@ namespace sparrow
fixed_width_binary_traits::value_type,
fixed_width_binary_traits::const_reference>;

namespace copy_tracker
{
template <>
inline std::string key<fixed_width_binary_array>()
{
return "fixed_width_binary_array";
}
}

template <std::ranges::sized_range T, typename CR, typename Ext>
struct array_inner_types<fixed_width_binary_array_impl<T, CR, Ext>> : array_inner_types_base
{
Expand Down Expand Up @@ -179,6 +189,14 @@ namespace sparrow
*/
explicit fixed_width_binary_array_impl(arrow_proxy);

fixed_width_binary_array_impl(const self_type&);

/** Move constructor */
fixed_width_binary_array_impl(self_type&&) noexcept = default;

self_type& operator=(const self_type&) = default;
self_type& operator=(self_type&&) noexcept = default;

/**
* @brief Generic constructor for creating fixed-width binary array.
*
Expand Down Expand Up @@ -655,6 +673,16 @@ namespace sparrow
SPARROW_ASSERT_TRUE(this->get_arrow_proxy().data_type() == data_type::FIXED_WIDTH_BINARY);
}

template <std::ranges::sized_range T, typename CR, typename Ext>
fixed_width_binary_array_impl<T, CR, Ext>::fixed_width_binary_array_impl(const self_type& rhs)
: base_type(rhs)
, m_element_size(rhs.m_element_size)
{
copy_tracker::increase(copy_tracker::key<self_type>());
}

// Move constructor and assignment are defaulted in the class definition

template <std::ranges::sized_range T, typename CR, typename Ext>
template <mpl::char_like C, validity_bitmap_input VB, input_metadata_container METADATA_RANGE>
arrow_proxy fixed_width_binary_array_impl<T, CR, Ext>::create_proxy(
Expand Down
12 changes: 12 additions & 0 deletions include/sparrow/layout/primitive_array_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "sparrow/arrow_interface/arrow_array.hpp"
#include "sparrow/arrow_interface/arrow_schema.hpp"
#include "sparrow/buffer/dynamic_bitset/dynamic_bitset.hpp"
#include "sparrow/debug/copy_tracker.hpp"
#include "sparrow/layout/array_bitmap_base.hpp"
#include "sparrow/layout/array_wrapper.hpp"
#include "sparrow/layout/primitive_data_access.hpp"
Expand All @@ -31,6 +32,16 @@ namespace sparrow
template <trivial_copyable_type T, typename Ext = empty_extension, trivial_copyable_type T2 = T>
class primitive_array_impl;

namespace copy_tracker
{
template <typename T>
requires mpl::is_type_instance_of_v<T, primitive_array_impl>
std::string key()
{
return "primitive_array";
}
}

template <trivial_copyable_type T, typename Ext, trivial_copyable_type T2>
struct array_inner_types<primitive_array_impl<T, Ext, T2>> : array_inner_types_base
{
Expand Down Expand Up @@ -482,6 +493,7 @@ namespace sparrow
: base_type(rhs)
, access_class_type(this->get_arrow_proxy(), DATA_BUFFER_INDEX)
{
copy_tracker::increase(copy_tracker::key<self_type>());
}

template <trivial_copyable_type T, typename Ext, trivial_copyable_type T2>
Expand Down
Loading
Loading