Skip to content

Commit caaa297

Browse files
authored
refactor: Bulk loading performance regression (#707)
Fix #702 Fix #703 Fix #704 FIx #705 Fix #706
1 parent e0f2e85 commit caaa297

4 files changed

Lines changed: 562 additions & 139 deletions

File tree

include/neug/common/columns/value_columns.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ class ValueColumnBuilder : public IContextColumnBuilder {
122122
}
123123

124124
inline void push_back_opt(const T& val) { data_.push_back(val); }
125+
inline void push_back_opt(T&& val) { data_.push_back(std::move(val)); }
125126
inline void push_back_null() override {
126127
if (valid_.empty()) {
127128
valid_.reserve(data_.capacity());

include/neug/storages/graph/vertex_table.h

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515
#pragma once
1616

17+
#include "neug/common/columns/value_columns.h"
1718
#include "neug/common/types/value.h"
1819
#include "neug/storages/graph/schema.h"
1920
#include "neug/storages/graph/vertex_timestamp.h"
@@ -270,8 +271,18 @@ class VertexTable {
270271
size_t row_num = pk_col->size();
271272
std::vector<vid_t> vids;
272273
vids.resize(row_num);
274+
bool is_string = std::is_same_v<PK_T, std::string_view> ||
275+
std::is_same_v<PK_T, std::string>;
276+
// Fast path: direct data() access when the column is a ValueColumn<PK_T>,
277+
// avoiding per-element virtual dispatch via get_elem().
278+
auto value_col = std::dynamic_pointer_cast<ValueColumn<PK_T>>(pk_col);
273279
for (size_t j = 0; j < row_num; ++j) {
274-
auto oid = pk_col->get_elem(j);
280+
Value oid;
281+
if (value_col) {
282+
oid = Value::CreateValue<PK_T>(value_col->data()[j]);
283+
} else {
284+
oid = pk_col->get_elem(j);
285+
}
275286
if (NEUG_UNLIKELY(indexer_->get_index(oid, vids[j]))) {
276287
if (NEUG_UNLIKELY(v_ts_->IsVertexValid(vids[j], MAX_TIMESTAMP))) {
277288
vids[j] = std::numeric_limits<vid_t>::max();
@@ -280,8 +291,6 @@ class VertexTable {
280291
}
281292
continue;
282293
}
283-
bool is_string = std::is_same_v<PK_T, std::string_view> ||
284-
std::is_same_v<PK_T, std::string>;
285294
vids[j] = insert_vertex_pk(oid, 0, is_string);
286295
}
287296
return vids;
@@ -290,8 +299,8 @@ class VertexTable {
290299
template <typename PK_T>
291300
void insert_vertices_impl(std::shared_ptr<IDataChunkSupplier> supplier) {
292301
auto row_nums = supplier->RowNum();
293-
if (row_nums <= 0) {
294-
LOG(WARNING) << "Row number from supplier is negative, treat it as 0.";
302+
if (row_nums < 0) {
303+
VLOG(1) << "Row number from supplier is unknown, skip pre-reserve.";
295304
row_nums = 0;
296305
}
297306
size_t new_size = indexer_->size() + row_nums;

src/storages/graph/edge_table.cc

Lines changed: 85 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <string_view>
2828
#include <utility>
2929

30+
#include "neug/common/columns/value_columns.h"
3031
#include "neug/storages/checkpoint_manager.h"
3132
#include "neug/storages/csr/csr_view_utils.h"
3233
#include "neug/storages/csr/immutable_csr.h"
@@ -210,10 +211,20 @@ void insert_edges_bundled_typed_impl(
210211
edge_data.reserve(src_lid.size());
211212
size_t cur_index = 0;
212213
for (auto& col : data_cols) {
213-
for (size_t i = 0; i < col->size(); ++i) {
214-
if (valid_flags[cur_index++]) {
215-
auto val = col->get_elem(i);
216-
edge_data.push_back(val.template GetValue<EDATA_T>());
214+
auto value_col = std::dynamic_pointer_cast<ValueColumn<EDATA_T>>(col);
215+
if (value_col) {
216+
for (size_t i = 0; i < value_col->size(); ++i) {
217+
if (valid_flags[cur_index++]) {
218+
edge_data.push_back(value_col->get_value(i));
219+
}
220+
}
221+
} else {
222+
// Fallback: virtual dispatch path (non-ValueColumn, rare).
223+
for (size_t i = 0; i < col->size(); ++i) {
224+
if (valid_flags[cur_index++]) {
225+
auto val = col->get_elem(i);
226+
edge_data.push_back(val.template GetValue<EDATA_T>());
227+
}
217228
}
218229
}
219230
}
@@ -234,15 +245,58 @@ void insert_edges_separated_impl(TypedCsrBase<uint64_t>* out_csr,
234245
in_csr->batch_put_edges(dst_lid, src_lid, edge_data);
235246
}
236247

237-
static std::vector<Value> get_row_from_data_chunks(
238-
const std::vector<std::shared_ptr<IContextColumn>>& prop_cols,
239-
size_t row_idx) {
240-
std::vector<Value> row;
241-
row.reserve(prop_cols.size());
242-
for (auto& col : prop_cols) {
243-
row.push_back(col->get_elem(row_idx));
248+
/// Type-erased inserter: writes ValueColumn<T>::data()[src_idx] to
249+
/// TypedColumn<T>::set_value(dst_idx), bypassing get_elem() + set_any().
250+
struct TypedColumnInserter {
251+
const IContextColumn* src;
252+
ColumnBase* dst;
253+
void (*fn)(const TypedColumnInserter&, size_t dst_idx, size_t src_idx,
254+
bool insert_safe);
255+
256+
inline void insert(size_t dst_idx, size_t src_idx, bool insert_safe) const {
257+
fn(*this, dst_idx, src_idx, insert_safe);
258+
}
259+
};
260+
261+
/// Fixed-length types: direct set_value, no Value, no virtual dispatch.
262+
/// Null entries already have T() in data_, so set_value writes the same
263+
/// default that set_any would write for null.
264+
template <typename T>
265+
void insert_typed_impl(const TypedColumnInserter& ins, size_t dst_idx,
266+
size_t src_idx, bool /*insert_safe*/) {
267+
auto* typed_dst = static_cast<TypedColumn<T>*>(ins.dst);
268+
auto vc = static_cast<const ValueColumn<T>*>(ins.src);
269+
typed_dst->set_value(dst_idx, vc->data()[src_idx]);
270+
}
271+
272+
/// Varchar: source is ValueColumn<std::string>, dest is
273+
/// TypedColumn<std::string_view>. Needs set_any for buffer resize, but skips
274+
/// get_elem() virtual dispatch.
275+
void insert_varchar_impl(const TypedColumnInserter& ins, size_t dst_idx,
276+
size_t src_idx, bool insert_safe) {
277+
auto* typed_dst = static_cast<TypedColumn<std::string_view>*>(ins.dst);
278+
auto vc = static_cast<const ValueColumn<std::string>*>(ins.src);
279+
typed_dst->set_any(dst_idx,
280+
Value::CreateValue<std::string>(vc->data()[src_idx]),
281+
insert_safe);
282+
}
283+
284+
TypedColumnInserter make_inserter(const DataType& type,
285+
const IContextColumn* src, ColumnBase* dst) {
286+
switch (type.id()) {
287+
#define MAKE_INSERTER(enum_val, cpp_type) \
288+
case DataTypeId::enum_val: \
289+
return {src, dst, &insert_typed_impl<cpp_type>};
290+
FOR_EACH_DATA_TYPE_NO_STRING(MAKE_INSERTER)
291+
#undef MAKE_INSERTER
292+
case DataTypeId::kVarchar:
293+
return {src, dst, &insert_varchar_impl};
294+
default:
295+
THROW_NOT_SUPPORTED_EXCEPTION(
296+
"Unsupported data type for column inserter: " +
297+
std::to_string(static_cast<int>(type.id())));
298+
return {};
244299
}
245-
return row;
246300
}
247301

248302
void batch_add_unbundled_edges_impl(
@@ -266,13 +320,30 @@ void batch_add_unbundled_edges_impl(
266320
if (c)
267321
prop_cols.push_back(c);
268322
}
323+
// Pre-resolve typed inserters for each column (once per chunk).
324+
std::vector<TypedColumnInserter> inserters;
325+
inserters.reserve(prop_cols.size());
326+
for (size_t j = 0; j < prop_cols.size(); ++j) {
327+
inserters.push_back(make_inserter(prop_types[j], prop_cols[j].get(),
328+
table_->get_column_by_id(j)));
329+
}
330+
// Pre-compute valid rows: column-major loop needs branch-free inner loop.
331+
std::vector<size_t> valid_rows;
332+
valid_rows.reserve(num_rows);
269333
for (size_t i = 0; i < num_rows; ++i) {
270334
assert(cur_index < valid_flags.size());
271335
if (valid_flags[cur_index++]) {
272-
auto row = get_row_from_data_chunks(prop_cols, i);
273-
table_->insert(offset++, row, true);
336+
valid_rows.push_back(i);
337+
}
338+
}
339+
// Column-major: same fn pointer per inner iteration (BTB-friendly),
340+
// sequential dst writes (cache-friendly), no branch.
341+
for (auto& ins : inserters) {
342+
for (size_t k = 0; k < valid_rows.size(); ++k) {
343+
ins.insert(offset + k, valid_rows[k], true);
274344
}
275345
}
346+
offset += valid_rows.size();
276347
}
277348
}
278349

0 commit comments

Comments
 (0)