Skip to content

GH-49133: [C++] Optimize SmallVector move assignment for static storage#49134

Closed
HyukjinKwon wants to merge 1 commit intoapache:mainfrom
HyukjinKwon:remove-non-actionable-todo
Closed

GH-49133: [C++] Optimize SmallVector move assignment for static storage#49134
HyukjinKwon wants to merge 1 commit intoapache:mainfrom
HyukjinKwon:remove-non-actionable-todo

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Feb 4, 2026

Rationale for this change

TBD

What changes are included in this PR?

TBD

Are these changes tested?

TBD

Are there any user-facing changes?

TBD

@HyukjinKwon HyukjinKwon force-pushed the remove-non-actionable-todo branch from f230c28 to 2aa870a Compare February 4, 2026 00:57
storage_type::move_construct_several(other.static_data_, static_data_, size_, N);
} else {
// Reset to static storage (in case we had dynamic storage before destroy())
data_ = static_data_;
Copy link
Member Author

@HyukjinKwon HyukjinKwon Feb 4, 2026

Choose a reason for hiding this comment

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

This is a piggyback bug fix that is technically not related to this optimization. I found out while benchmarking. I can separate it out if anyone prefers it.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Feb 4, 2026
@HyukjinKwon HyukjinKwon marked this pull request as draft February 4, 2026 01:05
@HyukjinKwon HyukjinKwon force-pushed the remove-non-actionable-todo branch from 2aa870a to 27aa86c Compare February 4, 2026 02:06
@HyukjinKwon HyukjinKwon marked this pull request as ready for review February 4, 2026 02:06
@HyukjinKwon HyukjinKwon force-pushed the remove-non-actionable-todo branch from 27aa86c to 1e26cd6 Compare February 4, 2026 02:11
@HyukjinKwon HyukjinKwon marked this pull request as draft February 4, 2026 02:16
@HyukjinKwon
Copy link
Member Author

Alright. Seems like the current usages in Arrow itself won't benefit a lot and I don't think this is worthwhile. I will create a new PR (to keep the current implementation as a reference) to remove this TODO.

@HyukjinKwon HyukjinKwon closed this Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant