Skip to content

Implement resize!(...; first=true) - #57313

Merged
oscardssmith merged 30 commits into
JuliaLang:masterfrom
ameligrana:patch-5
Sep 25, 2026
Merged

oscardssmith merged 30 commits into
JuliaLang:masterfrom
ameligrana:patch-5

Conversation

@ameligrana

Copy link
Copy Markdown
Contributor

Fixes #26326

@inkydragon inkydragon added the arrays [a, r, r, a, y, s] label Feb 8, 2025
@nsajko nsajko added the feature Indicates new feature / enhancement requests label Feb 8, 2025
@ameligrana

ameligrana commented Feb 8, 2025 •

Copy link
Copy Markdown
Contributor Author

Actually there is not a lot of consensus on the name in #26326. The options nonetheless could be maybe:

  • resizefirst!(a, n)
  • resize!(a, n, p) where p indicates where the resize! should start at, with 1 as resizefirst!, something similar to @StefanKarpinski comment

the second is also appealing, the first seems better to me because of simmetry with other functions e.g. pushfirst! and popfirst! as @oschulz pointed out. The options are actually also compatible in my opinion.

@LilithHafner LilithHafner added the triage This should be discussed on a triage call label Feb 8, 2025
@LilithHafner

Copy link
Copy Markdown
Member

IMO this is good functionality to publicize, tagging triage to discuss a new feature and to discuss naming.

@oscardssmith

Copy link
Copy Markdown
Member

Triage thinks this should be spelled resize(... first=true) (to be consistent with the new sizehint! kwarg in 1.11), but we otherwise like it.

@oscardssmith oscardssmith removed the triage This should be discussed on a triage call label Feb 13, 2025
@ameligrana ameligrana changed the title Implement resizefirst! Implement resize(..., first=true) Feb 14, 2025
@ameligrana ameligrana changed the title Implement resize(..., first=true) Implement resize!(..., first=true) Feb 14, 2025
@ameligrana

Copy link
Copy Markdown
Contributor Author

The failures seem unrelated, let me know if the implementation is okay now

@LilithHafner LilithHafner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks pretty good, I left a few suggestions.

Comment thread base/bitarray.jl Outdated
Comment thread base/array.jl
@LilithHafner LilithHafner changed the title Implement resize!(..., first=true) Implement resize!(...; first=true) Feb 16, 2025

@LilithHafner LilithHafner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IDK if an optimization that only applies when the resize amount is a multiple of 64 is actually worth implementing, especially if the resulting performance quirk is not documented.

Comment thread base/bitarray.jl Outdated
@ameligrana

Copy link
Copy Markdown
Contributor Author

IDK if an optimization that only applies when the resize amount is a multiple of 64 is actually worth implementing, especially if the resulting performance quirk is not documented.

to me it seems possibly good because someone interested in such a performance optimization (consider that the optimized version is orders of magnitudes faster than the other) would probably look at the implementation of resize! and try to adapt the code to exploit it

Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
@ameligrana

Copy link
Copy Markdown
Contributor Author

Also, it seems possible to me to apply this optimization for any n but it requires more global changes (which I'm not sure if they will slowdown other parts of the code though), so this could be the easiest step without perf overhead of that direction

@ameligrana

Copy link
Copy Markdown
Contributor Author

If someone else has any opinion on the bitvector optimization I'd like to hear you, so that we decide to keep it or not, I think that it is a good idea but @LilithHafner disagrees (at least a bit)

@DilumAluthge

Copy link
Copy Markdown
Member

This PR has had no activity for over six months.

@ameligrana Are you still interested in working on this PR?

If you're no longer interested in working on this PR, no worries! Just let me know, and I can close it.

@ameligrana

Copy link
Copy Markdown
Contributor Author

I think this just need to reach some consensus on some of the changes i.e. if keeping the bitarray optimization is worth it or not. I'll just wait some more to know what someone else thinks of it, and I'll split the PR into two if none intervenes

@vtjnash vtjnash added merge me PR is reviewed. When all tests are passing merge, making sure commit message is good. and removed status: waiting for PR author labels Sep 25, 2026
@ameligrana

Copy link
Copy Markdown
Contributor Author

I remove the multiple of 64 optimization (will create a new PR with that) and fixed conflicts, so I think it should be ready to be merged

@oscardssmith
oscardssmith merged commit 9649870 into JuliaLang:master Sep 25, 2026
12 checks passed
@IanButterworth IanButterworth removed the merge me PR is reviewed. When all tests are passing merge, making sure commit message is good. label Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] feature Indicates new feature / enhancement requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A function to resize vectors in front?

8 participants