Skip to content

Prevent vector reallocations by preallocating known sizes#127

Open
BogdanW3 wants to merge 3 commits into
stijnherfst:mainfrom
BogdanW3:vector_prealloc
Open

Prevent vector reallocations by preallocating known sizes#127
BogdanW3 wants to merge 3 commits into
stijnherfst:mainfrom
BogdanW3:vector_prealloc

Conversation

@BogdanW3
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/base/binary_reader.ixx Outdated
throw std::out_of_range("Trying to read out of range of buffer");
}
std::vector<T> result(reinterpret_cast<T*>(&buffer[position]), reinterpret_cast<T*>(&buffer[position]) + size);
std::vector<T> result(size);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Isn't this an extra initialization of the data?
std::vector<T> result(size); allocates size elements and value initializes them and then we copy them in.
The previous one would skip the value initialization?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What worries me here is that the iterator-based approach won't know to preallocate the correct size, so it would then resize the vector multiple times, actually not completely sure about this one

Copy link
Copy Markdown
Owner

@stijnherfst stijnherfst Mar 3, 2026

Choose a reason for hiding this comment

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

I checked the implemention (MSVC STL) and it will do one allocation when it's supplied with two iterators. So this would unfortunately be a pessimization.

}
}

return BinaryReader(std::move(buffer));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you expand a bit on why the above would be faster/better?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similar idea as with the binary_reader, preallocating the vector of the full size to avoid any re-allocating.

To be fair I might be going off on an unconfirmed suspicion that the iterator approach will realloc

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Okay, I did some research and this could theoretically be faster. I'm not sure if it is though so perhaps a benchmark would be a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants