Skip to content

Load performance regression: parse_endpoint_column get_elem() virtual dispatch overhead #705

Description

@zhanglei1949

Problem

Commit a281c0dd (replace Arrow CSV/JSON IO with native reader framework) changed parse_endpoint_column from direct typed array access to virtual dispatch via get_elem(), adding per-edge overhead for both src and dst endpoint lookups.

Root Cause

In edge_table.cc:184-190, parse_endpoint_column now goes through IContextColumn::get_elem() for every edge endpoint:

static void parse_endpoint_column(const IndexerType& indexer,
                                  const std::shared_ptr<IContextColumn>& col,
                                  std::vector<vid_t>& lids) {
  for (size_t i = 0; i < col->size(); ++i) {
    auto val = col->get_elem(i);        // ← virtual dispatch + Value construction
    auto vid = indexer.get_index(val);  // ← Value-based index lookup
    lids.push_back(vid);
  }
}

This is called twice per edge (src + dst), so the overhead is 2 × edge_count virtual dispatches + Value constructions.

v0.1.3 comparison

The old Arrow-based code dispatched on Arrow type and directly accessed the typed array:

if (array->type()->Equals(arrow::int64())) {
    auto casted = std::static_pointer_cast<arrow::Int64Array>(array);
    for (int64_t i = 0; i < casted->length(); ++i) {
        auto vid = indexer.get_index(execution::Value::INT64(casted->Value(i)));
        lids.push_back(vid);
    }
}

No virtual dispatch per element — direct memory read from the Arrow array.

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<T> and access data() directly.

Impact

Estimated ~500-1000s of the remaining ~4108s regression on SF1000. Impact is higher than vertex PK path because edges have two endpoint lookups per row.

Proposed Fix

Cast col to ValueColumn<T> based on the indexer's key type and access data() directly:

// Dispatch based on column type, then access data() directly
if (auto vc = std::dynamic_pointer_cast<ValueColumn<int64_t>>(col)) {
    const auto& data = vc->data();
    for (size_t i = 0; i < data.size(); ++i) {
        auto vid = indexer.get_index(Value::INT64(data[i]));
        lids.push_back(vid);
    }
} else if (auto vc = std::dynamic_pointer_cast<ValueColumn<std::string>>(col)) {
    // handle string PKs
} else {
    // fallback to get_elem()
}

Benchmark

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

References

Metadata

Metadata

Assignees

Labels

executorExecution engineperformancePerformance issue or improvementstoreStorage layer

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions