Skip to content

Fix #5312: use an aligned allocator.#5457

Closed
pitrou wants to merge 10 commits intonumpy:mainfrom
pitrou:align_alloc
Closed

Fix #5312: use an aligned allocator.#5457
pitrou wants to merge 10 commits intonumpy:mainfrom
pitrou:align_alloc

Conversation

@pitrou
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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)

@anirudh2290
Copy link
Copy Markdown
Member

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

@seberg
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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