Skip to content

Commit

Permalink
Array: remove private member _owns_memory_for_data
Browse files Browse the repository at this point in the history
no longer needed as we're using a shared<T[]> and can test that.
  • Loading branch information
KrisThielemans committed Feb 12, 2024
1 parent 6e0e47e commit 15791a0
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 23 deletions.
4 changes: 1 addition & 3 deletions src/include/stir/VectorWithOffset.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ class VectorWithOffset
swap(first.begin_allocated_memory, second.begin_allocated_memory);
swap(first.end_allocated_memory, second.end_allocated_memory);
swap(first.pointer_access, second.pointer_access);
swap(first._owns_memory_for_data, second._owns_memory_for_data);
swap(first.allocated_memory_sptr, second.allocated_memory_sptr);
}

Expand Down Expand Up @@ -227,7 +226,7 @@ class VectorWithOffset
//! change the range of the vector, new elements are set to \c T()
/*! New memory is allocated if the range grows outside the range specified by
get_capacity_min_index() till get_capacity_max_index(). Data is then copied
and old memory deallocated (unless owns_memory_for_data() is false).
and old memory deallocated (unless it is shared).
\todo in principle reallocation could be avoided when the new range would fit in the
old one by shifting.
Expand Down Expand Up @@ -421,7 +420,6 @@ class VectorWithOffset
//! boolean to test if get_data_ptr is called
// This variable is declared mutable such that get_const_data_ptr() can change it.
mutable bool pointer_access;
bool _owns_memory_for_data;
};

END_NAMESPACE_STIR
Expand Down
25 changes: 6 additions & 19 deletions src/include/stir/VectorWithOffset.inl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ void
VectorWithOffset<T>::init(const int min_index, const int max_index, T* const data_ptr, bool copy_data)
{
this->pointer_access = false;
this->_owns_memory_for_data = copy_data;
if (copy_data)
{
this->resize(min_index, max_index);
Expand All @@ -68,7 +67,7 @@ template <class T>
bool
VectorWithOffset<T>::owns_memory_for_data() const
{
return this->_owns_memory_for_data;
return this->allocated_memory_sptr ? true : false;
}

/*!
Expand All @@ -87,10 +86,7 @@ VectorWithOffset<T>::check_state() const
assert(begin_allocated_memory <= num + start);
assert(end_allocated_memory >= begin_allocated_memory);
assert(static_cast<unsigned>(end_allocated_memory - begin_allocated_memory) >= length);
if (allocated_memory_sptr)
{
// assert(_owns_memory_for_data);
}
assert(!allocated_memory_sptr || (allocated_memory_sptr.get() == begin_allocated_memory));
}

template <class T>
Expand All @@ -103,11 +99,7 @@ VectorWithOffset<T>::_destruct_and_deallocate()
// we'll have to be careful to delete only initialised elements
// and just de-allocate the rest

// Check on capacity probably not really necessary
// as begin_allocated_memory is == 0 in that case, and delete[] 0 doesn't do anything
// (I think). Anyway, we're on the safe side now...
if (this->owns_memory_for_data() && this->capacity() != 0)
this->allocated_memory_sptr = nullptr;
this->allocated_memory_sptr = nullptr;
}

template <class T>
Expand Down Expand Up @@ -251,9 +243,8 @@ VectorWithOffset<T>::rend() const

template <class T>
VectorWithOffset<T>::VectorWithOffset()
: _owns_memory_for_data(true)
: pointer_access(false)
{
pointer_access = false;
this->init();
}

Expand All @@ -266,8 +257,7 @@ template <class T>
VectorWithOffset<T>::VectorWithOffset(const int min_index, const int max_index)
: length(static_cast<unsigned>(max_index - min_index) + 1),
start(min_index),
pointer_access(false),
_owns_memory_for_data(true)
pointer_access(false)
{
if (max_index >= min_index)
{
Expand All @@ -291,7 +281,6 @@ VectorWithOffset<T>::VectorWithOffset(const int min_index, const int max_index,
: length(static_cast<unsigned>(max_index - min_index) + 1),
start(min_index),
pointer_access(false),
_owns_memory_for_data(false),
allocated_memory_sptr(nullptr)
{
this->begin_allocated_memory = data_ptr;
Expand Down Expand Up @@ -387,7 +376,6 @@ VectorWithOffset<T>::reserve(const int new_capacity_min_index, const int new_cap
allocated_memory_sptr = std::move(new_allocated_memory_sptr);
begin_allocated_memory = allocated_memory_sptr.get();
end_allocated_memory = begin_allocated_memory + new_capacity;
_owns_memory_for_data = true;
num = begin_allocated_memory + extra_at_the_left - (length > 0 ? start : 0);
this->check_state();
}
Expand Down Expand Up @@ -512,8 +500,7 @@ VectorWithOffset<T>::operator=(const VectorWithOffset& il)

template <class T>
VectorWithOffset<T>::VectorWithOffset(const VectorWithOffset& il)
: pointer_access(false),
_owns_memory_for_data(true)
: pointer_access(false)
{
this->init();
*this = il; // Uses assignment operator (above)
Expand Down
1 change: 0 additions & 1 deletion src/test/test_Array.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ ArrayTests::run_tests()
// auto mem = mem_sptr.get();
// std::copy(test.begin_all_const(), test.end_all_const(), mem);
// Array<1,float> preallocated(test.get_index_range(), mem_sptr, false);
check(!preallocated.owns_memory_for_data(), "test preallocated without copy: should not own memory");
check_if_equal(test, preallocated, "test preallocated: equality");
std::copy(test.begin_all_const(), test.end_all_const(), preallocated.begin_all());
check_if_equal(test, preallocated, "test preallocated: copy with full_iterator");
Expand Down

0 comments on commit 15791a0

Please sign in to comment.