8000 VTable Unification Docs by gatesn · Pull Request #6735 · vortex-data/vortex · GitHub
[go: up one dir, main page]

Skip to content

VTable Unification Docs#6735

Merged
gatesn merged 8 commits intodevelopfrom
ngates/vtable-plan
Mar 2, 2026
Merged

VTable Unification Docs#6735
gatesn merged 8 commits intodevelopfrom
ngates/vtable-plan

Conversation

@gatesn
Copy link
Contributor
@gatesn gatesn commented Mar 2, 2026

Updates the VTable plan to remove the FooInner<V> struct. So now Foo<V> is not internally Arc'd.

Downsides:

  • We lose the ability to go from &Foo<V> -> FooRef with only an Arc::clone. Turns out we were never doing this anyway, and where we do need return self.clone() it happens to be on the type-erased API which remains internally arc'd.

Upsides:

  • Borrowed downcasts as_::<V>() -> &Foo<V> can now return the full Foo wrapper struct, instead of just the associated type (-> &V::Metadata). This is a nicer API for users wanting to access typed fields instead of having to separately access shared properties from the erased object.

gatesn and others added 8 commits February 24, 2026 14:32
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Co-authored-by: Andrew Duffy <a10y@users.noreply.github.com>
Signed-off-by: Nicholas Gates <gatesn@users.noreply.github.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
@gatesn gatesn added the changelog/docs A docs change label Mar 2, 2026
@codspeed-hq
Copy link
codspeed-hq bot commented Mar 2, 2026

Merging this PR will improve performance by 12.36%

⚡ 2 improved benchmarks
✅ 952 untouched benchmarks
⏩ 1466 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation map_each[BufferMut<i32>, 128] 858.1 ns 770.6 ns +11.36%
Simulation bitwise_not_vortex_buffer_mut[128] 530.3 ns 471.9 ns +12.36%

Comparing ngates/vtable-plan (682710d) with develop (7b5b719)

Open in CodSpeed

Footnote 8000 s

  1. 1466 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.

@gatesn gatesn requested a review from connortsui20 March 2, 2026 03:21
@gatesn gatesn enabled auto-merge (squash) March 2, 2026 21:07
Copy link
Contributor
@connortsui20 connortsui20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gatesn gatesn merged commit 85bcaaa into develop Mar 2, 2026
51 of 52 checks passed
@gatesn gatesn deleted the ngates/vtable-plan branch March 2, 2026 21:07
@gatesn gatesn mentioned this pull request Mar 2, 2026
joseph-isaacs pushed a commit that referenced this pull request Mar 4, 2026
As per #6544 and the migration plan in
#6735

---------

Signed-off-by: Nicholas Gates <nick@nickgates.com>
fastio pushed a commit to fastio/vortex that referenced this pull request Mar 10, 2026
Updates the VTable plan to remove the `FooInner<V>` struct. So now
`Foo<V>` is not internally Arc'd.

Downsides:
* We lose the ability to go from `&Foo<V> -> FooRef` with only an
Arc::clone. Turns out we were never doing this anyway, and where we _do_
need return self.clone() it happens to be on the type-erased API which
remains internally arc'd.

Upsides:
* Borrowed downcasts `as_::<V>() -> &Foo<V>` can now return the full
`Foo` wrapper struct, instead of just the associated type (`->
&V::Metadata`). This is a nicer API for users wanting to access typed
fields instead of having to separately access shared properties from the
erased object.

---------

Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <gatesn@users.noreply.github.com>
Co-authored-by: Andrew Duffy <a10y@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/docs A docs change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0