Skip to content

Fix #5312: use an aligned allocator. #5457

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
wants to merge 10 commits into from
Closed

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Jan 16, 2015

Introduce two new functions, get_data_alignment() and set_data_alignment() which allow
setting the guaranteed alignment at runtime.

Introduce two new functions, get_data_alignment() and set_data_alignment() which allow
setting the guaranteed alignment at runtime.
@juliantaylor
Copy link
Contributor

an issue is that with this pointers can't be freed with free anymore, so technically an ABI break.
Its probably unlikely but there might be users out there who try that, or pass in malloc pointers to array constructors and then transfer ownership to the array.

@pitrou
Copy link
Member Author

pitrou commented Jan 16, 2015

I noticed that PyDataMem_RENEW() is obviously wrong the base pointer changes alignment. Fixing, but this adds some overhead in cases where the base offset changes.
(also, there seem to be some problems in 32-bit mode)

Conflicts:
	numpy/core/src/multiarray/multiarraymodule.c
@anirudh2290
Copy link
Member

This PR and #5470 needs another look from reviewers, @seberg we probably need a "Awaits review" tag for such PRs ?

@seberg
Copy link
Member

seberg commented May 21, 2020

There is a "ready for review" tag, not sure if it helps here... I guess the problem is getting someone to review who knows this better.
Not sure I am too worried about transferred ownership of allocated pointers to consider it an ABI break). I suppose if we go there, that also opens us up to simply using the Python allocator for the strides+shape (dims), or even moving it into the object allocation to optimize it away entirely?

Anyway, you are right, this is one that could be reactivate, but may need prodding reviewers a bit :).

@anirudh2290
Copy link
Member

okay thanks for the summary @seberg. I guess I will tag this and the other pr Ready for Review and revisit this later.

@seberg
Copy link
Member

seberg commented Sep 8, 2021

Thanks for this! I will close it, since it is now superseded by gh-17582 and NEP 49 — Data allocation strategies.

@seberg seberg closed this Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants