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

Several bug fixes for serialization or nested/repeated messages #386

Conversation

jasonmreding
Copy link
Collaborator

Description

This PR fixes several issues with message serialization when EfficientMessageCopy = TRUE.
#378
#384

Implementation

  • Fixed several places where the array being allocated was larger than required. In effect, we were requesting an allocation for the total byte size when NumericArrayResize takes the element size and not the byte size.
  • Fixed a few places where arrays were always allocating 8 byte pointers rather than 4 bytes for a pointer on 32 bit platforms.
  • Cleaned up some state management data that wasn't really needed since the lifetime of the data was effectively managed by the stack frame.
  • Fixed issues where the buffer used to copy data for repeated nested messages was always copying previous elements in the array and copying the address for the same buffer multiple times in the message structure. This leads to DWARNs in the memory manager since we attempt to deallocate the same block of memory multiple times. The general idea is that we allocate a buffer once that is reused, and the size of the buffer is doubled on demand as needed when copying data. Each repeated message element then starts copying data back at element zero rather than starting from the index in the buffer where the previous element left off.

Testing

I manually tested the proto file attached to #378 failed before the changes and works as expected afterwards. I also verified the proto API referenced from #384 works as expected and no longer crashes using either 32 or 64 bit LV on Windows.

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.

1 participant