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.

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

Single PE allocation API #1088

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wrrobin
Copy link
Collaborator

@wrrobin wrrobin commented Mar 29, 2023

This PR introduces the single PE allocation API and related changes. New APIs being added are as below.

void * shmemx_malloc_onepe(size_t size);
void * shmemx_calloc_onepe(size_t count, size_t size);
ptrdiff_t shmemx_ptrdiff_of_ptr(void *ptr);
void *shmemx_ptr_of_ptrdiff(ptrdiff_t ptrdiff);
void shmemx_free_onepe(void *ptr);

A simple unit test is used to test the APIs.

@wrrobin
Copy link
Collaborator Author

wrrobin commented Mar 29, 2023

@stewartl318 - I was trying to add you as a reviewer on this, but not able to. I am trying this way to send a notification.

@stewartl318
Copy link
Collaborator

As a general comment, I am personally in favor of alloc_single and in favor of the address translation idea, but opposed to varsize.

I think that varsize clutters up the API without providing any benefits.

The alternative is just to allocate the full requested size on every PE, but not put real memory behind it. This could result in runtime failures if the user tries to put and get to memory that is not there, but in the varsize case, there are no checks to verify that addresses are actually inside the varsize region anyway, so it is no different.

So, what is the precise benefit of varsize that justifies an API change?

There is also an argument that alloc_single can equally well be dealt with by using sbrk to expand the globals and statics region, and fcollect _end.

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.

2 participants