Skip to content

Do not zero-allocate the memory in pzstd::vector - #354

Merged
kevlu8 merged 2 commits into
mainfrom
dont-alloc-vector
Aug 7, 2026
Merged

Do not zero-allocate the memory in pzstd::vector#354
kevlu8 merged 2 commits into
mainfrom
dont-alloc-vector

Conversation

@kevlu8

@kevlu8 kevlu8 commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Should be a slight speedup, somewhere around ~5%

Elo   | 8.13 +- 5.06 (95%)
SPRT  | 4.0+0.04s Threads=1 Hash=32MB
LLR   | 2.93 (-2.25, 2.89) [-4.00, 0.00]
Games | N: 5126 W: 1342 L: 1222 D: 2562
Penta | [26, 559, 1276, 673, 29]

https://ob.int0x80.ca/test/979/

Bench: 2186605

@kevlu8
kevlu8 requested a review from wdotmathree August 7, 2026 19:15

@wdotmathree wdotmathree left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

All comments aren't important to fix due to the fact that all T being used are trivially constructable. However they will become legitimate correctness bugs if other values of T are used in the future.

Comment thread engine/pzstl/vector.hpp Outdated

vector() : sz(0) {}

void push_back(T value) noexcept {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Takes value by value. Should have 2 signatures: one taking const reference, and one taking rvalue.

Comment thread engine/pzstl/vector.hpp Outdated
vector() : sz(0) {}

void push_back(T value) noexcept {
data[sz++] = value;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Calls assignment operator instead of constructor.

Comment thread engine/pzstl/vector.hpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't call destructor.

Comment thread engine/pzstl/vector.hpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't call destructor.

Comment thread engine/pzstl/vector.hpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Takes value by value. Should take const reference.

Bench: 2186656
@kevlu8
kevlu8 force-pushed the dont-alloc-vector branch from 58dbef1 to 5dfd3dc Compare August 7, 2026 20:31
@kevlu8
kevlu8 merged commit 2d86998 into main Aug 7, 2026
9 checks passed
@kevlu8
kevlu8 deleted the dont-alloc-vector branch August 7, 2026 20:39
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