Skip to content

potential performance improvement: specialize copying constructor #43

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.

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

Closed
nsajko opened this issue Apr 25, 2024 · 3 comments · Fixed by #136
Closed

potential performance improvement: specialize copying constructor #43

nsajko opened this issue Apr 25, 2024 · 3 comments · Fixed by #136
Assignees

Comments

@nsajko
Copy link
Collaborator

nsajko commented Apr 25, 2024

Didn't check if a significant performance improvement would be possible, however it crossed my mind that it might possibly make sense to implement the FixedSizeArray copying constructor via copy(::Memory). Right now it dispatches to the converting constructor for AbstractArray.

@giordano
Copy link
Collaborator

giordano commented Jun 1, 2025

Was this fixed by #128?

@nsajko nsajko self-assigned this Jun 1, 2025
@nsajko
Copy link
Collaborator Author

nsajko commented Jun 1, 2025

The idea with this issue, although not clear in the OP, is to have special methods as an optimization that are variations on this, but possibly also for other constructors:

function FixedSizeArray(a::FixedSizeArray)
    copy(a)
end

That said, now the constructors forward to collect_as, so it would be more general to add such methods to the Collect callable, after #123 is done.

@nsajko
Copy link
Collaborator Author

nsajko commented Jun 3, 2025

Turns out this doesn't help performance directly, it just helps by allowing effect inference to infer nothrow. Making a PR.

nsajko added a commit to nsajko/FixedSizeArrays.jl that referenced this issue Jun 3, 2025
Helps effect inference infer `nothrow`.

Fixes JuliaArrays#43
@nsajko nsajko closed this as completed in 9f3017c Jun 4, 2025
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 a pull request may close this issue.

2 participants