Skip to content

Load performance regression: insert_edges_bundled_typed_impl get_elem()+GetValue() overhead #706

Description

@zhanglei1949

Problem

Commit a281c0dd (replace Arrow CSV/JSON IO with native reader framework) changed insert_edges_bundled_typed_impl from direct typed array access to get_elem() + GetValue() virtual dispatch, adding per-edge overhead for bundled edge data.

Root Cause

In edge_table.cc:217-233, insert_edges_bundled_typed_impl now goes through get_elem() + GetValue() for every edge:

template <typename EDATA_T>
void insert_edges_bundled_typed_impl(
    TypedCsrBase<EDATA_T>* out_csr, TypedCsrBase<EDATA_T>* in_csr,
    const std::vector<vid_t>& src_lid, const std::vector<vid_t>& dst_lid,
    const std::vector<std::shared_ptr<IContextColumn>>& data_cols,
    const std::vector<bool>& valid_flags) {
  std::vector<EDATA_T> edge_data;
  edge_data.reserve(src_lid.size());
  size_t cur_index = 0;
  for (auto& col : data_cols) {
    for (size_t i = 0; i < col->size(); ++i) {
      if (valid_flags[cur_index++]) {
        auto val = col->get_elem(i);                          // ← virtual dispatch + Value
        edge_data.push_back(val.template GetValue<EDATA_T>()); // ← extract from Value
      }
    }
  }
  out_csr->batch_put_edges(src_lid, dst_lid, edge_data);
  in_csr->batch_put_edges(dst_lid, src_lid, edge_data);
}

Two overheads per edge: virtual get_elem() call + Value::GetValue<EDATA_T>().

v0.1.3 comparison

The old Arrow-based code directly accessed the typed Arrow array:

auto casted = std::static_pointer_cast<ARROW_COL_T>(array);
for (int64_t i = 0; i < casted->length(); ++i) {
    if (valid_flags[cur_index++]) {
        edge_data.push_back(EDATA_T(casted->Value(i)));  // direct memory read
    }
}

No virtual dispatch, no intermediate Value object.

Same pattern as #694

This is the same class of overhead that was already fixed for set_properties_from_context_column in #694 (b0905a1). The fix pattern is identical: cast to ValueColumn<EDATA_T> and access data() directly.

Impact

Estimated ~300-500s of the remaining ~4108s regression on SF1000. Only applies to bundled edges (single property column); unbundled edges use a different path (batch_add_unbundled_edges_impl).

Proposed Fix

Cast col to ValueColumn<EDATA_T> and access data() directly:

for (auto& col : data_cols) {
    if (auto vc = std::dynamic_pointer_cast<ValueColumn<EDATA_T>>(col)) {
        const auto& data = vc->data();
        for (size_t i = 0; i < data.size(); ++i) {
            if (valid_flags[cur_index++]) {
                edge_data.push_back(data[i]);
            }
        }
    } else {
        // fallback to get_elem() + GetValue()
    }
}

Benchmark

v0.1.3:          15166.20s
main (b0905a1):  19273.97s  (+4108s remaining)

References

Metadata

Metadata

Assignees

Labels

performancePerformance issue or improvementstoreStorage layer

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions