Skip to content

Commit 1fba296

Browse files
aalkinaalkin
authored
DPL Analysis: modernize and cleanup some code (#14975)
1 parent b332613 commit 1fba296

File tree

9 files changed

+129
-172
lines changed

9 files changed

+129
-172
lines changed

‎Framework/AnalysisSupport/src/AODReaderHelpers.cxx‎

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ struct Buildable {
3737
std::vector<o2::soa::IndexRecord> records;
3838
std::shared_ptr<arrow::Schema> outputSchema;
3939

40-
Buildable(InputSpec const& spec)
40+
explicit Buildable(InputSpec const& spec)
4141
: binding{spec.binding}
4242
{
4343
auto&& [origin_, description_, version_] = DataSpecUtils::asConcreteDataMatcher(spec);
@@ -58,9 +58,8 @@ struct Buildable {
5858
}
5959
outputSchema = std::make_shared<arrow::Schema>([](std::vector<o2::soa::IndexRecord> const& recs) {
6060
std::vector<std::shared_ptr<arrow::Field>> fields;
61-
for (auto& r : recs) {
62-
fields.push_back(r.field());
63-
}
61+
fields.reserve(recs.size());
62+
std::ranges::transform(recs, std::back_inserter(fields), [](auto& r) { return r.field(); });
6463
return fields;
6564
}(records))
6665
->WithMetadata(std::make_shared<arrow::KeyValueMetadata>(std::vector{std::string{"label"}}, std::vector{std::string{binding}}));
@@ -87,19 +86,12 @@ AlgorithmSpec AODReaderHelpers::indexBuilderCallback(ConfigContext const& /*ctx*
8786
{
8887
return AlgorithmSpec::InitCallback{[](InitContext& ic) {
8988
auto const& requested = ic.services().get<DanglingEdgesContext>().requestedIDXs;
90-
std::vector<Buildable> buildables;
91-
for (auto const& i : requested) {
92-
buildables.emplace_back(i);
93-
}
9489
std::vector<Builder> builders;
95-
for (auto& b : buildables) {
96-
builders.push_back(b.createBuilder());
97-
}
90+
builders.reserve(requested.size());
91+
std::ranges::transform(requested, std::back_inserter(builders), [](auto const& i) { return Buildable{i}.createBuilder(); });
9892
return [builders](ProcessingContext& pc) mutable {
9993
auto outputs = pc.outputs();
100-
for (auto& builder : builders) {
101-
outputs.adopt(Output{builder.origin, builder.description, builder.version}, builder.materialize(pc));
102-
}
94+
std::ranges::for_each(builders, [&pc, &outputs](auto& builder) { outputs.adopt(Output{builder.origin, builder.description, builder.version}, builder.materialize(pc)); });
10395
};
10496
}};
10597
}
@@ -119,7 +111,7 @@ struct Spawnable {
119111
header::DataDescription description;
120112
header::DataHeader::SubSpecificationType version;
121113

122-
Spawnable(InputSpec const& spec)
114+
explicit Spawnable(InputSpec const& spec)
123115
: binding{spec.binding}
124116
{
125117
auto&& [origin_, description_, version_] = DataSpecUtils::asConcreteDataMatcher(spec);
@@ -144,16 +136,19 @@ struct Spawnable {
144136
iws.str(json);
145137
schemas.emplace_back(ArrowJSONHelpers::read(iws));
146138
}
147-
for (auto const& i : spec.metadata | views::filter_string_params_starts_with("input:") | std::ranges::views::transform([](auto const& param) {
148-
return DataSpecUtils::fromMetadataString(param.defaultValue.template get<std::string>());
149-
})) {
150-
matchers.emplace_back(std::get<ConcreteDataMatcher>(i.matcher));
151-
}
139+
std::ranges::transform(spec.metadata |
140+
views::filter_string_params_starts_with("input:") |
141+
std::ranges::views::transform(
142+
[](auto const& param) {
143+
return DataSpecUtils::fromMetadataString(param.defaultValue.template get<std::string>());
144+
}),
145+
std::back_inserter(matchers), [](auto const& i) { return std::get<ConcreteDataMatcher>(i.matcher); });
152146

153147
std::vector<std::shared_ptr<arrow::Field>> fields;
154-
for (auto& s : schemas) {
155-
std::copy(s->fields().begin(), s->fields().end(), std::back_inserter(fields));
156-
}
148+
std::ranges::for_each(schemas,
149+
[&fields](auto const& s) {
150+
std::ranges::copy(s->fields(), std::back_inserter(fields));
151+
});
157152

158153
inputSchema = std::make_shared<arrow::Schema>(fields);
159154
expressions = expressions::materializeProjectors(projectors, inputSchema, outputSchema->fields());
@@ -194,20 +189,12 @@ AlgorithmSpec AODReaderHelpers::aodSpawnerCallback(ConfigContext const& /*ctx*/)
194189
{
195190
return AlgorithmSpec::InitCallback{[](InitContext& ic) {
196191
auto const& requested = ic.services().get<DanglingEdgesContext>().spawnerInputs;
197-
std::vector<Spawnable> spawnables;
198-
for (auto const& i : requested) {
199-
spawnables.emplace_back(i);
200-
}
201192
std::vector<Spawner> spawners;
202-
for (auto& s : spawnables) {
203-
spawners.push_back(s.createMaker());
204-
}
205-
193+
spawners.reserve(requested.size());
194+
std::ranges::transform(requested, std::back_inserter(spawners), [](auto const& i) { return Spawnable{i}.createMaker(); });
206195
return [spawners](ProcessingContext& pc) mutable {
207196
auto outputs = pc.outputs();
208-
for (auto& spawner : spawners) {
209-
outputs.adopt(Output{spawner.origin, spawner.description, spawner.version}, spawner.materialize(pc));
210-
}
197+
std::ranges::for_each(spawners, [&pc, &outputs](auto& spawner) { outputs.adopt(Output{spawner.origin, spawner.description, spawner.version}, spawner.materialize(pc)); });
211198
};
212199
}};
213200
}

‎Framework/Core/include/Framework/ASoA.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,7 @@ struct TableIterator : IP, C... {
12831283
};
12841284

12851285
struct ArrowHelpers {
1286+
static std::shared_ptr<arrow::Table> joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables);
12861287
static std::shared_ptr<arrow::Table> joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables, std::span<const char* const> labels);
12871288
static std::shared_ptr<arrow::Table> joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables, std::span<const std::string> labels);
12881289
static std::shared_ptr<arrow::Table> concatTables(std::vector<std::shared_ptr<arrow::Table>>&& tables);

‎Framework/Core/include/Framework/Expressions.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ std::shared_ptr<gandiva::Projector> createProjectorHelper(size_t nColumns, expre
712712
std::shared_ptr<arrow::Schema> schema,
713713
std::vector<std::shared_ptr<arrow::Field>> const& fields);
714714

715-
std::vector<std::shared_ptr<gandiva::Expression>> materializeProjectors(std::vector<expressions::Projector> const& projectors, std::shared_ptr<arrow::Schema> const& inputSchema, std::vector<std::shared_ptr<arrow::Field>> outputFields);
715+
std::vector<std::shared_ptr<gandiva::Expression>> materializeProjectors(std::vector<expressions::Projector> const& projectors, std::shared_ptr<arrow::Schema> const& inputSchema, std::vector<std::shared_ptr<arrow::Field>> const& outputFields);
716716

717717
template <typename... C>
718718
std::shared_ptr<gandiva::Projector> createProjectors(framework::pack<C...>, std::vector<std::shared_ptr<arrow::Field>> const& fields, gandiva::SchemaPtr schema)

‎Framework/Core/src/ASoA.cxx‎

Lines changed: 47 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -62,71 +62,71 @@ SelectionVector sliceSelection(std::span<int64_t const> const& mSelectedRows, in
6262
auto start_iterator = std::lower_bound(mSelectedRows.begin(), mSelectedRows.end(), start);
6363
auto stop_iterator = std::lower_bound(start_iterator, mSelectedRows.end(), end);
6464
SelectionVector slicedSelection{start_iterator, stop_iterator};
65-
std::transform(slicedSelection.begin(), slicedSelection.end(), slicedSelection.begin(),
66-
[&start](int64_t idx) {
67-
return idx - static_cast<int64_t>(start);
68-
});
65+
std::ranges::transform(slicedSelection.begin(), slicedSelection.end(), slicedSelection.begin(),
66+
[&start](int64_t idx) {
67+
return idx - static_cast<int64_t>(start);
68+
});
6969
return slicedSelection;
7070
}
7171

72-
std::shared_ptr<arrow::Table> ArrowHelpers::joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables, std::span<const char* const> labels)
72+
std::shared_ptr<arrow::Table> ArrowHelpers::joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables)
7373
{
74-
if (tables.size() == 1) {
75-
return tables[0];
76-
}
77-
for (auto i = 0U; i < tables.size() - 1; ++i) {
78-
if (tables[i]->num_rows() != tables[i + 1]->num_rows()) {
79-
throw o2::framework::runtime_error_f("Tables %s and %s have different sizes (%d vs %d) and cannot be joined!",
80-
labels[i], labels[i + 1], tables[i]->num_rows(), tables[i + 1]->num_rows());
81-
}
82-
}
8374
std::vector<std::shared_ptr<arrow::Field>> fields;
8475
std::vector<std::shared_ptr<arrow::ChunkedArray>> columns;
85-
86-
for (auto& t : tables) {
87-
auto tf = t->fields();
88-
std::copy(tf.begin(), tf.end(), std::back_inserter(fields));
89-
}
90-
91-
auto schema = std::make_shared<arrow::Schema>(fields);
92-
93-
if (tables[0]->num_rows() != 0) {
94-
for (auto& t : tables) {
95-
auto tc = t->columns();
96-
std::copy(tc.begin(), tc.end(), std::back_inserter(columns));
76+
bool notEmpty = (tables[0]->num_rows() != 0);
77+
std::ranges::for_each(tables, [&fields, &columns, notEmpty](auto const& t) {
78+
std::ranges::copy(t->fields(), std::back_inserter(fields));
79+
if (notEmpty) {
80+
std::ranges::copy(t->columns(), std::back_inserter(columns));
9781
}
98-
}
82+
});
83+
auto schema = std::make_shared<arrow::Schema>(fields);
9984
return arrow::Table::Make(schema, columns);
10085
}
10186

102-
std::shared_ptr<arrow::Table> ArrowHelpers::joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables, std::span<const std::string> labels)
87+
namespace
88+
{
89+
template <typename T>
90+
requires(std::same_as<T, std::string>)
91+
auto makeString(T const& str)
92+
{
93+
return str.c_str();
94+
}
95+
template <typename T>
96+
requires(std::same_as<T, const char*>)
97+
auto makeString(T const& str)
98+
{
99+
return str;
100+
}
101+
102+
template <typename T>
103+
void canNotJoin(std::vector<std::shared_ptr<arrow::Table>> const& tables, std::span<T> labels)
103104
{
104-
if (tables.size() == 1) {
105-
return tables[0];
106-
}
107105
for (auto i = 0U; i < tables.size() - 1; ++i) {
108106
if (tables[i]->num_rows() != tables[i + 1]->num_rows()) {
109107
throw o2::framework::runtime_error_f("Tables %s and %s have different sizes (%d vs %d) and cannot be joined!",
110-
labels[i].c_str(), labels[i + 1].c_str(), tables[i]->num_rows(), tables[i + 1]->num_rows());
108+
makeString(labels[i]), makeString(labels[i + 1]), tables[i]->num_rows(), tables[i + 1]->num_rows());
111109
}
112110
}
113-
std::vector<std::shared_ptr<arrow::Field>> fields;
114-
std::vector<std::shared_ptr<arrow::ChunkedArray>> columns;
111+
}
112+
} // namespace
115113

116-
for (auto& t : tables) {
117-
auto tf = t->fields();
118-
std::copy(tf.begin(), tf.end(), std::back_inserter(fields));
114+
std::shared_ptr<arrow::Table> ArrowHelpers::joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables, std::span<const char* const> labels)
115+
{
116+
if (tables.size() == 1) {
117+
return tables[0];
119118
}
119+
canNotJoin(tables, labels);
120+
return joinTables(std::forward<std::vector<std::shared_ptr<arrow::Table>>>(tables));
121+
}
120122

121-
auto schema = std::make_shared<arrow::Schema>(fields);
122-
123-
if (tables[0]->num_rows() != 0) {
124-
for (auto& t : tables) {
125-
auto tc = t->columns();
126-
std::copy(tc.begin(), tc.end(), std::back_inserter(columns));
127-
}
123+
std::shared_ptr<arrow::Table> ArrowHelpers::joinTables(std::vector<std::shared_ptr<arrow::Table>>&& tables, std::span<const std::string> labels)
124+
{
125+
if (tables.size() == 1) {
126+
return tables[0];
128127
}
129-
return arrow::Table::Make(schema, columns);
128+
canNotJoin(tables, labels);
129+
return joinTables(std::forward<std::vector<std::shared_ptr<arrow::Table>>>(tables));
130130
}
131131

132132
std::shared_ptr<arrow::Table> ArrowHelpers::concatTables(std::vector<std::shared_ptr<arrow::Table>>&& tables)
@@ -135,7 +135,6 @@ std::shared_ptr<arrow::Table> ArrowHelpers::concatTables(std::vector<std::shared
135135
return tables[0];
136136
}
137137
std::vector<std::shared_ptr<arrow::ChunkedArray>> columns;
138-
assert(tables.size() > 1);
139138
std::vector<std::shared_ptr<arrow::Field>> resultFields = tables[0]->schema()->fields();
140139
auto compareFields = [](std::shared_ptr<arrow::Field> const& f1, std::shared_ptr<arrow::Field> const& f2) {
141140
// Let's do this with stable sorting.
@@ -165,13 +164,12 @@ std::shared_ptr<arrow::Table> ArrowHelpers::concatTables(std::vector<std::shared
165164
columns.push_back(std::make_shared<arrow::ChunkedArray>(chunks));
166165
}
167166

168-
auto result = arrow::Table::Make(std::make_shared<arrow::Schema>(resultFields), columns);
169-
return result;
167+
return arrow::Table::Make(std::make_shared<arrow::Schema>(resultFields), columns);
170168
}
171169

172170
arrow::ChunkedArray* getIndexFromLabel(arrow::Table* table, std::string_view label)
173171
{
174-
auto field = std::find_if(table->schema()->fields().begin(), table->schema()->fields().end(), [&](std::shared_ptr<arrow::Field> const& f) {
172+
auto field = std::ranges::find_if(table->schema()->fields(), [&](std::shared_ptr<arrow::Field> const& f) {
175173
auto caseInsensitiveCompare = [](const std::string_view& str1, const std::string& str2) {
176174
return std::ranges::equal(
177175
str1, str2,

‎Framework/Core/src/AnalysisHelpers.cxx‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,44 +46,32 @@ void IndexBuilder::resetBuilders(std::vector<framework::IndexColumnBuilder>& bui
4646
std::shared_ptr<arrow::Table> IndexBuilder::materialize(std::vector<framework::IndexColumnBuilder>& builders, std::vector<std::shared_ptr<arrow::Table>>&& tables, std::vector<soa::IndexRecord> const& records, std::shared_ptr<arrow::Schema> const& schema, bool exclusive)
4747
{
4848
auto size = tables[0]->num_rows();
49-
if (builders.empty()) {
49+
if (O2_BUILTIN_UNLIKELY(builders.empty())) {
5050
builders = makeBuilders(std::move(tables), records);
5151
} else {
5252
resetBuilders(builders, std::move(tables));
5353
}
5454

55-
std::vector<bool> finds;
56-
finds.resize(builders.size());
5755
for (int64_t counter = 0; counter < size; ++counter) {
5856
int64_t idx = -1;
5957
if (std::get<framework::SelfBuilder>(builders[0].builder).keyIndex == nullptr) {
6058
idx = counter;
6159
} else {
6260
idx = std::get<framework::SelfBuilder>(builders[0].builder).keyIndex->valueAt(counter);
6361
}
64-
for (auto i = 0U; i < builders.size(); ++i) {
65-
finds[i] = builders[i].find(idx);
66-
}
67-
if (exclusive) {
68-
if (std::none_of(finds.begin(), finds.end(), [](bool const x) { return x == false; })) {
69-
builders[0].fill(counter);
70-
for (auto i = 1U; i < builders.size(); ++i) {
71-
builders[i].fill(idx);
72-
}
73-
}
74-
} else {
62+
63+
bool found = true;
64+
std::ranges::for_each(builders, [&idx, &found](auto& builder) { found &= builder.find(idx); });
65+
66+
if (!exclusive || found) {
7567
builders[0].fill(counter);
76-
for (auto i = 1U; i < builders.size(); ++i) {
77-
builders[i].fill(idx);
78-
}
68+
std::ranges::for_each(builders.begin() + 1, builders.end(), [&idx](auto& builder) { builder.fill(idx); });
7969
}
8070
}
8171

8272
std::vector<std::shared_ptr<arrow::ChunkedArray>> arrays;
8373
arrays.reserve(builders.size());
84-
for (auto& builder : builders) {
85-
arrays.push_back(builder.result());
86-
}
74+
std::ranges::transform(builders, std::back_inserter(arrays), [](auto& builder) { return builder.result(); });
8775

8876
return arrow::Table::Make(schema, arrays);
8977
}
@@ -142,9 +130,7 @@ std::shared_ptr<arrow::Table> spawnerHelper(std::shared_ptr<arrow::Table> const&
142130
}
143131

144132
arrays.reserve(nColumns);
145-
for (auto i = 0U; i < nColumns; ++i) {
146-
arrays.push_back(std::make_shared<arrow::ChunkedArray>(chunks[i]));
147-
}
133+
std::ranges::transform(chunks, std::back_inserter(arrays), [](auto&& chunk) { return std::make_shared<arrow::ChunkedArray>(chunk); });
148134

149135
return arrow::Table::Make(newSchema, arrays);
150136
}
@@ -188,9 +174,8 @@ std::string serializeIndexRecords(std::vector<o2::soa::IndexRecord>& irs)
188174
std::vector<std::shared_ptr<arrow::Table>> extractSources(ProcessingContext& pc, std::vector<ConcreteDataMatcher> const& matchers)
189175
{
190176
std::vector<std::shared_ptr<arrow::Table>> tables;
191-
for (auto const& matcher : matchers) {
192-
tables.emplace_back(pc.inputs().get<TableConsumer>(matcher)->asArrowTable());
193-
}
177+
tables.reserve(matchers.size());
178+
std::ranges::transform(matchers, std::back_inserter(tables), [&pc](auto const& matcher) { return pc.inputs().get<TableConsumer>(matcher)->asArrowTable(); });
194179
return tables;
195180
}
196181

0 commit comments

Comments
 (0)