Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: rewrite Stack::push_slice to allow arbitrary lengths #812

Merged
merged 1 commit into from
Oct 25, 2023
Merged

refactor: rewrite Stack::push_slice to allow arbitrary lengths #812

merged 1 commit into from
Oct 25, 2023

Conversation

DaniPopes
Copy link
Collaborator

@DaniPopes DaniPopes commented Oct 16, 2023 •
edited
Loading

Performance is the exact same. The current implementation is a hand-rolled version of this PR's.

@DaniPopes DaniPopes force-pushed the stack-arbitrary-push branch 2 times, most recently from da8c12c to 3780848 Compare October 16, 2023 21:09
@rakita
Copy link
Member

rakita commented Oct 18, 2023

How did you measure performance for this? It looks great btw!

@DaniPopes DaniPopes force-pushed the stack-arbitrary-push branch 3 times, most recently from c4f6845 to b5005ca Compare October 25, 2023 16:35
@DaniPopes DaniPopes force-pushed the stack-arbitrary-push branch from b5005ca to cdd4754 Compare October 25, 2023 16:37
@DaniPopes
Copy link
Collaborator Author

DaniPopes commented Oct 25, 2023 •
edited
Loading

How did you measure performance for this?

Manually looked at generated assembly for push{1,2,20,32} and it's identical minus register names.

Also ran callgrind:

before (885c0cc): 416,256,326
after (cdd4754): 416,256,326

@rakita rakita merged commit 0d78d1e into bluealloy:main Oct 25, 2023
@DaniPopes DaniPopes deleted the stack-arbitrary-push branch October 25, 2023 17:03
@DaniPopes DaniPopes mentioned this pull request Oct 25, 2023
Merged
@github-actions github-actions bot mentioned this pull request Jan 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DaniPopes @rakita