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

Alignment verification for both ConcreteBuffer and BufferExpander #307

Closed
tigercosmos opened this issue Apr 8, 2024 · 2 comments
Closed
Assignees
Labels
array Multi-dimensional array implementation enhancement New feature or request performance Profiling, runtime, and memory consumption postpone Need not to be done for now

Comments

@tigercosmos
Copy link
Collaborator

Ensure alignment verification for both ConcreteBuffer and BufferExpander. Upon casting the raw buffer to typed data, it is imperative to validate if the alignment meets the required alignment. This alignment validation should be implemented without utilizing a template and must be enforced within the buffer's allocator.

Original discussion from #304 (comment)

@yungyuc yungyuc added performance Profiling, runtime, and memory consumption array Multi-dimensional array implementation labels Apr 9, 2024
@yungyuc yungyuc moved this from Todo to In Progress in tabular data processing Jun 4, 2024
@tigercosmos
Copy link
Collaborator Author

@yungyuc I think for a while, I cannot think of a case in the memory that could be not in alignment. Currently, SimpleCollector holds a BufferExpander, and SimpleCollector never exchanges the expander with another SimpleCollector in another type.

For example, SimpleCollector<int> will not exchange the underlying expander with the one of SimpleCollector<double>.

In addition, a SimpleCollector will never expand the buffer with a different alignment of the buffer. Therefore, I wonder if this issue is still valid. Do you have any thoughts on this?

@yungyuc
Copy link
Member

yungyuc commented Jun 5, 2024

When implementing BufferExpander, I planned to align the memory at allocation. It calls an abstraction layer for multi-platform code. On mac the alignment allocation is unnecessary because malloc() is always aligned.

The alignment may be a thing for GPU. But I agree we do not need to do anything about it because we do not even have a GPU CI runner.

If you do not think alignment is a thing, we can close this issue for now. Let me do it.

@yungyuc yungyuc closed this as completed Jun 5, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in tabular data processing Jun 5, 2024
@yungyuc yungyuc added postpone Need not to be done for now enhancement New feature or request labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
array Multi-dimensional array implementation enhancement New feature or request performance Profiling, runtime, and memory consumption postpone Need not to be done for now
Projects
Status: Done
Development

No branches or pull requests

2 participants