ARROW-8022: [C++] Add static and small vector implementations#10915
ARROW-8022: [C++] Add static and small vector implementations#10915pitrou wants to merge 17 commits into
Conversation
|
Some operations are not yet implemented: |
|
@ursabot please benchmark |
|
Benchmark runs are scheduled for baseline = e544927 and contender = eef6764ec58c823c98ad1df9072039c7e04f9c4e. Results will be available as each benchmark for each run completes. |
57c3c52 to
a97521c
Compare
f7c9dba to
9a1f2f0
Compare
9a1f2f0 to
b9a02e3
Compare
4c13a78 to
6e61e29
Compare
|
@ursabot please benchmark |
|
Benchmark runs are scheduled for baseline = 973dbae and contender = afc3b92. Results will be available as each benchmark for each run completes. |
04de50a to
58ad05e
Compare
|
I added a |
58ad05e to
6b48d23
Compare
bkietz
left a comment
There was a problem hiding this comment.
Two nits. I think this is good to go
Thanks for doing this!
| storage_.bump_size(count); | ||
| auto* p = storage_.storage_ptr(); | ||
| for (size_t i = 0; i < count; ++i) { | ||
| p[i].construct(value); | ||
| } |
There was a problem hiding this comment.
I think we can reuse resize here
| storage_.bump_size(count); | |
| auto* p = storage_.storage_ptr(); | |
| for (size_t i = 0; i < count; ++i) { | |
| p[i].construct(value); | |
| } | |
| resize(count, value); |
There was a problem hiding this comment.
Same answer as below. Given that this code is short enough, I think it's reasonable to keep it as is.
| } | ||
|
|
||
| template <typename InputIt> | ||
| void assign_by_copying(size_t n, InputIt src) { |
There was a problem hiding this comment.
I think all four of these could be collapsed into a single SmallVectorImpl::assign(It begin, It end) method without overhead. Then we'd write
StaticVectorImpl& operator=(std::vector<T>&& other) noexcept {
assign(std::make_move_iterator(other.begin()), std::make_move_iterator(other.end()));
return *this;
}There was a problem hiding this comment.
Ideally without overhead, but compilers do not always do their best, which is why I've avoided generalization here.
No description provided.