Skip to content

Commit b689d6d

Browse files
committed
Fix empty reusable maps in ThreadPool
1 parent fbd962f commit b689d6d

File tree

4 files changed

+42
-20
lines changed

4 files changed

+42
-20
lines changed

src/openvic-simulation/types/FixedVector.hpp

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
#pragma once
22

3+
#include <algorithm>
34
#include <cassert>
45
#include <cstddef>
56
#include <memory>
67
#include <utility>
78

89
#include "openvic-simulation/core/template/Concepts.hpp"
910
#include "openvic-simulation/core/Typedefs.hpp"
11+
#include "openvic-simulation/utility/Logger.hpp"
1012

1113
namespace OpenVic::_detail {
1214
//fixed capacity + not movable + not copyable
@@ -19,6 +21,12 @@ namespace OpenVic::_detail {
1921
OV_NO_UNIQUE_ADDRESS Allocator _allocator;
2022
T* const _data_start_ptr;
2123

24+
explicit FixedVector(const size_t capacity)
25+
: _max_size(capacity),
26+
_size(0),
27+
_allocator(),
28+
_data_start_ptr(allocator_traits::allocate(_allocator, capacity)) {}
29+
2230
public:
2331
using const_reference = T const&;
2432
using difference_type = ptrdiff_t;
@@ -33,19 +41,23 @@ namespace OpenVic::_detail {
3341
constexpr T const* data() const { return _data_start_ptr; }
3442
constexpr bool empty() const { return _size == 0; }
3543

36-
explicit FixedVector(const size_t capacity)
37-
: _max_size(capacity),
38-
_size(0),
44+
FixedVector static create_empty(const size_t capacity) { return FixedVector(capacity); }
45+
46+
FixedVector(const size_t size, T const& value_for_all_indices)
47+
: _max_size(size),
48+
_size(size),
3949
_allocator(),
40-
_data_start_ptr(allocator_traits::allocate(_allocator, capacity)) {}
50+
_data_start_ptr(allocator_traits::allocate(_allocator, size)) {
51+
std::fill(_data_start_ptr, _data_start_ptr + size, value_for_all_indices);
52+
}
4153

4254
//Generator (size_t i) -> U (where T is constructable from U)
4355
template<typename GeneratorTemplateType>
4456
// The generator must NOT return a tuple
4557
requires (!specialization_of<std::remove_cvref_t<std::invoke_result_t<GeneratorTemplateType, size_t>>, std::tuple>)
4658
// The type must be constructible from the generator's single return value
4759
&& std::constructible_from<T, decltype(std::declval<GeneratorTemplateType>()(std::declval<size_t>()))>
48-
FixedVector(size_t size, GeneratorTemplateType&& generator)
60+
FixedVector(const size_t size, GeneratorTemplateType&& generator)
4961
: _max_size(size),
5062
_size(size),
5163
_allocator(),
@@ -74,7 +86,7 @@ namespace OpenVic::_detail {
7486
)
7587
};
7688
}
77-
FixedVector(size_t size, GeneratorTemplateType&& generator)
89+
FixedVector(const size_t size, GeneratorTemplateType&& generator)
7890
: _max_size(size),
7991
_size(size),
8092
_allocator(),
@@ -125,12 +137,22 @@ namespace OpenVic::_detail {
125137
const_reverse_iterator rend() const { return const_reverse_iterator(begin()); }
126138
const_reverse_iterator crend() const { return const_reverse_iterator(begin()); }
127139

128-
T& operator[](size_t index) {
129-
assert(index < _size && "Index out of bounds.");
140+
T& operator[](const size_t index) {
141+
if (OV_unlikely(_size <= index)) {
142+
spdlog::error_s(
143+
"Out of bound non-const indexing on FixedVector. index {} _size {}",
144+
index, _size
145+
);
146+
}
130147
return _data_start_ptr[index];
131148
}
132-
const T& operator[](size_t index) const {
133-
assert(index < _size && "Index out of bounds.");
149+
const T& operator[](const size_t index) const {
150+
if (OV_unlikely(_size <= index)) {
151+
spdlog::error_s(
152+
"Out of bound const indexing on FixedVector. index {} _size {}",
153+
index, _size
154+
);
155+
}
134156
return _data_start_ptr[index];
135157
}
136158

src/openvic-simulation/types/IndexedFlatMap.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ namespace OpenVic {
269269
) : keys(new_keys),
270270
min_index { new_keys.front().index },
271271
max_index { new_keys.back().index },
272-
values(new_keys.size()) {
272+
values(memory::FixedVector<ValueType>::create_empty(new_keys.size())) {
273273
static_assert(has_index<ForwardedKeyType>);
274274
if (!validate_new_keys(new_keys)) {
275275
return;
@@ -363,7 +363,7 @@ namespace OpenVic {
363363
) : keys(new_keys),
364364
min_index { type_safe::get(new_keys.front().index) },
365365
max_index { type_safe::get(new_keys.back().index) },
366-
values(new_keys.size()) {
366+
values(memory::FixedVector<ValueType>::create_empty(new_keys.size())) {
367367
static_assert(has_index<ForwardedKeyType>);
368368
if (!validate_new_keys(new_keys)) {
369369
return;
@@ -428,7 +428,7 @@ namespace OpenVic {
428428
: keys(new_keys),
429429
min_index { type_safe::get(new_keys.front().index) },
430430
max_index { type_safe::get(new_keys.back().index) },
431-
values(new_keys.size()) {
431+
values(memory::FixedVector<ValueType>::create_empty(new_keys.size())) {
432432
static_assert(has_index<ForwardedKeyType>);
433433
if (!validate_new_keys(new_keys)) {
434434
return;

src/openvic-simulation/utility/ThreadPool.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ void ThreadPool::loop_until_cancelled(
2626
) {
2727
IndexedFlatMap<GoodDefinition, char> reusable_goods_mask { good_keys };
2828

29-
memory::FixedVector<fixed_point_t> reusable_country_map_0 { country_keys.size() };
30-
memory::FixedVector<fixed_point_t> reusable_country_map_1 { country_keys.size() };
29+
memory::FixedVector<fixed_point_t> reusable_country_map_0 { country_keys.size(), fixed_point_t::_0 };
30+
memory::FixedVector<fixed_point_t> reusable_country_map_1 { country_keys.size(), fixed_point_t::_0 };
3131
TypedSpan<country_index_t, fixed_point_t> reusable_country_map_0_span { reusable_country_map_0 };
3232
TypedSpan<country_index_t, fixed_point_t> reusable_country_map_1_span { reusable_country_map_1 };
3333

tests/src/types/FixedVector.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ int DestructionCounter::destructor_count = 0;
6767
// A simple test case to check the basic constructor and accessors with a simple type.
6868
TEST_CASE("FixedVector Construction and basic accessors","[FixedVector]") {
6969
constexpr size_t capacity = 5;
70-
FixedVector<int> vec(capacity);
70+
FixedVector<int> vec = FixedVector<int>::create_empty(capacity);
7171

7272
// Initial state check
7373
CHECK(vec.size() == 0);
@@ -111,7 +111,7 @@ TEST_CASE("FixedVector Generator constructor (tuple)","[FixedVector]") {
111111
// Test emplace_back, pop_back, and clear with a complex type.
112112
TEST_CASE("FixedVector Manipulation (emplace_back, pop_back, clear)","[FixedVector]") {
113113
constexpr size_t capacity = 4;
114-
FixedVector<ComplexType> vec(capacity);
114+
FixedVector<ComplexType> vec = FixedVector<ComplexType>::create_empty(capacity);
115115

116116
// Emplace elements with multiple arguments
117117
vec.emplace_back(1, "hello");
@@ -153,7 +153,7 @@ TEST_CASE("FixedVector Manipulation (emplace_back, pop_back, clear)","[FixedVect
153153
// Test that accessor methods return references to the same memory location
154154
TEST_CASE("FixedVector Accessor reference consistency","[FixedVector]") {
155155
constexpr size_t capacity = 3;
156-
FixedVector<int> vec(capacity);
156+
FixedVector<int> vec = FixedVector<int>::create_empty(capacity);
157157

158158
vec.emplace_back(1);
159159
CHECK(&vec.front() == &vec.back());
@@ -171,7 +171,7 @@ TEST_CASE("FixedVector Accessor reference consistency","[FixedVector]") {
171171
// Test with a non-copyable and non-movable type to ensure in-place construction.
172172
TEST_CASE("FixedVector Non-copyable, non-movable type","[FixedVector]") {
173173
constexpr size_t capacity = 2;
174-
FixedVector<NonCopyableNonMovable> vec(capacity);
174+
FixedVector<NonCopyableNonMovable> vec = FixedVector<NonCopyableNonMovable>::create_empty(capacity);
175175

176176
// This should work because emplace_back constructs in place
177177
const int value0 = 1;
@@ -196,7 +196,7 @@ TEST_CASE("FixedVector Destruction, Clear, and Refill","[FixedVector]") {
196196

197197
{
198198
// Use a scope to test the FixedVector's destructor
199-
FixedVector<DestructionCounter> vec(capacity);
199+
FixedVector<DestructionCounter> vec = FixedVector<DestructionCounter>::create_empty(capacity);
200200

201201
// Emplace a few objects
202202
vec.emplace_back();

0 commit comments

Comments
 (0)