New C++ api by myrrc · Pull Request #8651 · vortex-data/vortex

myrrc · 2026-07-03T16:12:16Z

Replace C++ api with a new one which depends on vortex-ffi. Remove cxx from workspace. Port FFI examples and tests to C++ examples as tests.

Add vx_expression_clone to C FFI.
Remove const modifier from output from vx_expression_not

codspeed-hq · 2026-07-03T16:22:54Z

Merging this PR will not alter performance

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 3 improved benchmarks
❌ 1 regressed benchmark
✅ 1641 untouched benchmarks
⏩ 4 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`rebuild_naive`	91.2 µs	105.4 µs	-13.45%
⚡	WallTime	`cuda/bitpacked_u8/unpack/3bw[100M]`	350.4 µs	299.4 µs	+17.05%
⚡	Simulation	`chunked_varbinview_into_canonical[(100, 100)]`	307.5 µs	272.5 µs	+12.84%
⚡	Simulation	`bitwise_not_vortex_buffer_mut[128]`	273.6 ns	244.4 ns	+11.93%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing myrrc/cxx2 (de2512a) with develop (f1465af)²}

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on develop (596099c) during the generation of this report, so f1465af was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>

0ax1

first pass

0ax1 · 2026-07-08T15:53:08Z

@@ -490,20 +490,32 @@ jobs:
      - uses: ./.github/actions/setup-prebuild
        with:
          enable-sccache: "true"


We didn't delete vortex-cxx/src/read.rs?

0ax1 · 2026-07-08T15:53:26Z

@@ -475,8 +475,8 @@ jobs:
          command: check ${{ matrix.checks }}

  cxx-test:


Gotta fix up CI breaks.

0ax1 · 2026-07-08T15:55:20Z

@@ -0,0 +1,1152 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright the Vortex contributors
+#pragma once


Is this header checked by .clang-tidy? (and the src file)

0ax1 · 2026-07-08T15:56:54Z

@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: Apache-2.0
+# SPDX-FileCopyrightText: Copyright the Vortex contributors
+NamespaceIndentation: None


Why do we introduce a new formatting here? We have a ref formatting .clang-format at the repo top-level.

0ax1 · 2026-07-08T16:01:00Z

+};
+
+// 16-bit float
+struct f16 {


I guess we could detect at build time what C++ version the compiler supports and branch on using the C++23 std lib f16 or our custom one.

0ax1 · 2026-07-08T16:03:23Z

+
+// Types that a PrimitiveView can hold
+template <class T>
+concept view_type = primitive_type<T> || std::is_same_v<T, bool>;


primitive_view? view_type in isolation reads vague.

0ax1 · 2026-07-08T16:07:10Z

+     * Estimated count. Throws if not known(). For inexact estimates this is
+     * is an upper bound.
+     */
+    uint64_t value() const;


Not sure I understand this API. Why not just have value? What's the flow when the caller passes in the fallback. And why does that need to handled in a method of Estimate?

0ax1 · 2026-07-08T16:07:57Z

+
+    // List accessors. Valid only on List and FixedSizeList dtypes
+
+    DataType list_element() const;


Every public fn in this PR added needs docs.

0ax1 · 2026-07-08T16:09:02Z

+
+namespace dtype {
+
+inline constexpr bool Nullable = true;


What's the naming convention we wanna go for? Nullable reads like a type. Google convention is kNullable, using the k prefix for constants.

0ax1

second pass

0ax1 · 2026-07-08T16:17:20Z

@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: Apache-2.0
+# SPDX-FileCopyrightText: Copyright the Vortex contributors
+InheritParentConfig: true


Same here as for .clang-format. Whatever we do for vortex-cxx should be repo global I think.

0ax1 · 2026-07-08T16:20:03Z

+
+int main(int argc, char** argv) {
+    if (argc != 2) {
+        std::cerr << "Usage: dtype <file glob>\n";


nit: Isn't it Usage: <dtype> <file glob>\

0ax1 · 2026-07-08T16:22:00Z

+        fs::temp_directory_path() / "hello_vortex_example.vortex";
+    std::cout << "Writing to: " << out.string() << '\n';
+
+    Session session;


=> rename to VortexSession, given that session is a generic description?

0ax1 · 2026-07-08T16:25:19Z

+
+private:
+    void reset() noexcept {
+        if (!path_.empty()) {


Do we care about checking the error in tests?

0ax1 · 2026-07-08T16:51:56Z

+    return BytesView(std::move(canonical), std::move(validity), n);
+}
+
+Array make_struct(


Why actually not && in the signature for the return type?

0ax1 · 2026-07-08T16:54:43Z

+}
+
+void StructArrayBuilder::Deleter::operator()(
+    vx_struct_column_builder* ptr) const noexcept {


What's our rule of thumb for sprinkling noexcept?

0ax1 · 2026-07-08T16:55:29Z

+        bits_ = other.bits_;
+        bit_offset_ = other.bit_offset_;
+        all_invalid_ = other.all_invalid_;
+        other.owner_ = nullptr;


Why is = nullptr responsibility of the assignment operator?

0ax1 · 2026-07-08T16:56:19Z

+        }
+        session_ = std::move(other.session_);
+        stream_ = other.stream_;
+        other.stream_ = ArrowArrayStream{};


Also here we do why change other? other.stream_ = ArrowArrayStream{};

0ax1 · 2026-07-08T16:57:54Z

+#include <utility>
+#include <vector>
+
+/**


Same here, let's think about whether all of this should go into a single header. We can still have a convenience vortex.hpp header with only include statements.

myrrc added the changelog/break A breaking API change label Jul 3, 2026

myrrc force-pushed the myrrc/cxx2 branch 2 times, most recently from 05babc3 to 5795c45 Compare July 3, 2026 16:35

myrrc requested a review from 0ax1 July 3, 2026 16:47

myrrc force-pushed the myrrc/cxx2 branch 2 times, most recently from a044015 to a5c7567 Compare July 3, 2026 16:56

myrrc added the lang/cpp Relates to the Vortex C++ API label Jul 7, 2026

myrrc force-pushed the myrrc/cxx2 branch 7 times, most recently from 6b2036e to 66bfb31 Compare July 8, 2026 10:00

initial

de2512a

Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>

myrrc force-pushed the myrrc/cxx2 branch from 66bfb31 to de2512a Compare July 8, 2026 14:07

0ax1 reviewed Jul 8, 2026

View reviewed changes

vortex-data deleted a comment from github-actions Bot Jul 8, 2026

0ax1 reviewed Jul 8, 2026

View reviewed changes

		@@ -475,8 +475,8 @@ jobs:
		command: check ${{ matrix.checks }}

		cxx-test:


		// List accessors. Valid only on List and FixedSizeList dtypes

		DataType list_element() const;

Uh oh!

Conversation

myrrc commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Performance Changes

Footnotes

Uh oh!

0ax1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ax1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ax1 Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ax1 Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

myrrc commented Jul 3, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 3, 2026 •

edited

Loading

0ax1 Jul 8, 2026 •

edited

Loading

0ax1 Jul 8, 2026 •

edited

Loading