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

Fixed serialization of type requiring "size" information in uint32_t or uint64_t instead of size_t, added options::force_aligned_access #45

Merged
merged 6 commits into from
Jul 19, 2024

Conversation

lrodorigo
Copy link

@lrodorigo lrodorigo commented Jun 9, 2024

This pull request addresses the issue identified in #43 regarding the serialization of size information using size_t type, which is not architecture-agnostic.

To ensure consistent serialization across different architectures, the size information for strings, vectors, and other types that require a size attribute is now serialized using a type explicitly defined in alpaca::detail::size_t_serialized_type (uint32_t or uint64_t).
This change guarantees uniformity in the serialized data, irrespective of the platform.
I also aligned the tests and I added a set of test generated by chatgpt.

@lrodorigo lrodorigo changed the title Fixed serialization of type requiring "size" information in uint32_t or uint64_t instead of size_t Fixed serialization of type requiring "size" information in uint32_t or uint64_t instead of size_t, added options::force_aligned_access Jun 13, 2024
@lrodorigo
Copy link
Author

lrodorigo commented Jun 13, 2024

The Alpaca library, by default, utilizes unaligned memory access, as this is permitted on the x86_64 architecture.
However, certain architectures, such as the Arm Cortex-M3, M4, and M33, require aligned memory access 64-bit data types.
Other architectures (such as Cortex-M0) requires aligned access also for 32-bit data types.

For architectures where aligned memory access is necessary, I added
options::force_aligned_access option.
When this option is enabled, the library will not perform unaligned accesses for 64 bit and 32 bit types, and it will use memcpy instead.

@finger563
Copy link
Contributor

@lrodorigo - are you still working on this? I'm also needing these changes and was wondering if I could help update the PR to get the tests to pass :)

@finger563
Copy link
Contributor

I was able to get this branch to build and pass on macos (which in CI is failing):

Diff:
CleanShot 2024-07-19 at 09 38 07

Results:
CleanShot 2024-07-19 at 09 37 29

p-ranav added a commit that referenced this pull request Jul 19, 2024
Update of #45 (replace size_t with uint32_t for length encoding and add new option for aligned memory access)
@p-ranav p-ranav merged commit d3800c5 into p-ranav:master Jul 19, 2024
3 of 7 checks passed
@lrodorigo
Copy link
Author

lrodorigo commented Jul 19, 2024 via email

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 this pull request may close these issues.

None yet

4 participants