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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions src/cluster_copier.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ namespace grpc_labview {
case LVMessageMetadataType::SInt32Value:
CopySInt32ToCluster(fieldMetadata, start, value);
break;
case LVMessageMetadataType:: SInt64Value:
case LVMessageMetadataType::SInt64Value:
CopySInt64ToCluster(fieldMetadata, start, value);
break;
case LVMessageMetadataType::Fixed32Value:
CopyFixed32ToCluster(fieldMetadata, start, value);
break;
case LVMessageMetadataType::Fixed64Value:
CopyFixed64ToCluster(fieldMetadata, start, value);
break;
CopyFixed64ToCluster(fieldMetadata, start, value);
break;
case LVMessageMetadataType::SFixed32Value:
CopySFixed32ToCluster(fieldMetadata, start, value);
break;
Expand All @@ -95,13 +95,13 @@ namespace grpc_labview {
}
}

// second pass to fill the oneof selected_index. We can do this in one pass when we push the selected_field to the end of the oneof cluster!
// second pass to fill the oneof selected_index. We can do this in one pass when we push the selected_field to the end of the oneof cluster!
// TODO: Skip the entire loop if the message has no oneof. It's a bool in the metadata.
for (auto val : message._metadata->_mappedElements)
{
auto fieldMetadata = val.second;
if (fieldMetadata->isInOneof&& fieldMetadata->protobufIndex < 0)
{
auto fieldMetadata = val.second;
if (fieldMetadata->isInOneof && fieldMetadata->protobufIndex < 0)
{
// This field is the selected_index field of a oneof
if (oneof_containerToSelectedIndexMap.find(fieldMetadata->oneofContainerName) != oneof_containerToSelectedIndexMap.end())
{
Expand All @@ -119,7 +119,7 @@ namespace grpc_labview {
message._values.clear();
std::map<std::string, int> oneof_containerToSelectedIndexMap; // Needed to serialize only the field related to the selected_index
for (auto val : message._metadata->_mappedElements)
{
{
auto fieldMetadata = val.second;
if (fieldMetadata->isInOneof)
{
Expand All @@ -140,7 +140,7 @@ namespace grpc_labview {
if (fieldMetadata->protobufIndex >= 0)
{
auto it = oneof_containerToSelectedIndexMap.find(fieldMetadata->oneofContainerName);
assert (it != oneof_containerToSelectedIndexMap.end());
assert(it != oneof_containerToSelectedIndexMap.end());
auto selected_index = it->second;
if (selected_index != fieldMetadata->protobufIndex)
{
Expand Down Expand Up @@ -287,7 +287,7 @@ namespace grpc_labview {
auto repeatedString = static_cast<const LVRepeatedMessageValue<std::string>&>(*value);
if (repeatedString._value.size() != 0)
{
NumericArrayResize(0x08, 1, start, repeatedString._value.size());
NumericArrayResize(GetTypeCodeForSize(sizeof(char*)), 1, start, repeatedString._value.size());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: An 8 byte pointer is too large for 32 bit LV.

auto array = *(LV1DArrayHandle*)start;
(*array)->cnt = repeatedString._value.size();
int x = 0;
Expand Down Expand Up @@ -330,14 +330,21 @@ namespace grpc_labview {
{
auto nestedMetadata = repeatedNested->_value.front()->_metadata;
auto clusterSize = nestedMetadata->clusterSize;
auto byteSize = repeatedNested->_value.size() * clusterSize;
auto alignment = nestedMetadata->alignmentRequirement;
auto alignedElementSize = byteSize / alignment;
if (byteSize % alignment != 0)
{
alignedElementSize++;
}

NumericArrayResize(0x08, 1, start, repeatedNested->_value.size() * clusterSize);
NumericArrayResize(GetTypeCodeForSize(alignment), 1, start, alignedElementSize);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The previous allocation was 8x larger than it needed to be since the element size passed was the byte size, and we were specifying an element size of 8 bytes. We now pick an element size that meets our alignment requirements and update the number of elements to allocate accordingly.

auto array = *(LV1DArrayHandle*)start;
(*array)->cnt = repeatedNested->_value.size();
int x = 0;
for (auto str : repeatedNested->_value)
{
auto lvCluster = (LVCluster**)(*array)->bytes(x * clusterSize, nestedMetadata->alignmentRequirement);
auto lvCluster = (LVCluster**)(*array)->bytes(x * clusterSize, alignment);
*lvCluster = nullptr;
CopyToCluster(*str, (int8_t*)lvCluster);
x += 1;
Expand Down Expand Up @@ -419,7 +426,7 @@ namespace grpc_labview {
auto byteCount = count * sizeof(int32_t);
memcpy((*array)->bytes<int32_t>(), mappedArray, byteCount);
}

free(mappedArray);
}
else
Expand Down Expand Up @@ -810,7 +817,7 @@ namespace grpc_labview {
repeatedValue->_value.Reserve(count);
auto dest = repeatedValue->_value.AddNAlreadyReserved(count);
memcpy(dest, mappedArray, count * sizeof(int32_t));

free(mappedArray);
}
}
Expand Down
78 changes: 47 additions & 31 deletions src/lv_interop.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static int kCleanOnIdle = 2;
//---------------------------------------------------------------------
//---------------------------------------------------------------------
typedef int (*NumericArrayResize_T)(int32_t, int32_t, void* handle, size_t size);
typedef int (*PostLVUserEvent_T)(grpc_labview::LVUserEventRef ref, void *data);
typedef int (*PostLVUserEvent_T)(grpc_labview::LVUserEventRef ref, void* data);
typedef int (*Occur_T)(grpc_labview::MagicCookie occurrence);
typedef int32_t(*RTSetCleanupProc_T)(grpc_labview::CleanupProcPtr cleanUpProc, grpc_labview::gRPCid* id, int32_t mode);
typedef unsigned char** (*DSNewHandlePtr_T)(size_t);
Expand All @@ -44,14 +44,14 @@ namespace grpc_labview
{
grpc_labview::PointerManager<grpc_labview::gRPCid> gPointerManager;

//---------------------------------------------------------------------
// Allows for definition of the LVRT DLL path to be used for callback functions
// This function should be called prior to calling InitCallbacks()
//---------------------------------------------------------------------
void SetLVRTModulePath(std::string modulePath)
{
ModulePath = modulePath;
}
// Allows for definition of the LVRT DLL path to be used for callback functions
// This function should be called prior to calling InitCallbacks()
//---------------------------------------------------------------------
void SetLVRTModulePath(std::string modulePath)
{
ModulePath = modulePath;
}

#ifdef _WIN32

Expand All @@ -67,24 +67,24 @@ namespace grpc_labview
return;
}

HMODULE lvModule;

if(ModulePath != "")
{
lvModule = GetModuleHandle(ModulePath.c_str());
}
else
{
lvModule = GetModuleHandle("LabVIEW.exe");
if (lvModule == nullptr)
{
lvModule = GetModuleHandle("lvffrt.dll");
}
if (lvModule == nullptr)
{
lvModule = GetModuleHandle("lvrt.dll");
}
}
HMODULE lvModule;

if (ModulePath != "")
{
lvModule = GetModuleHandle(ModulePath.c_str());
}
else
{
lvModule = GetModuleHandle("LabVIEW.exe");
if (lvModule == nullptr)
{
lvModule = GetModuleHandle("lvffrt.dll");
}
if (lvModule == nullptr)
{
lvModule = GetModuleHandle("lvrt.dll");
}
}
NumericArrayResizeImp = (NumericArrayResize_T)GetProcAddress(lvModule, "NumericArrayResize");
PostLVUserEvent = (PostLVUserEvent_T)GetProcAddress(lvModule, "PostLVUserEvent");
Occur = (Occur_T)GetProcAddress(lvModule, "Occur");
Expand Down Expand Up @@ -131,15 +131,15 @@ namespace grpc_labview
//---------------------------------------------------------------------
//---------------------------------------------------------------------
int NumericArrayResize(int32_t typeCode, int32_t numDims, void* handle, size_t size)
{
{
return NumericArrayResizeImp(typeCode, numDims, handle, size);
}

//---------------------------------------------------------------------
//---------------------------------------------------------------------
int PostUserEvent(LVUserEventRef ref, void *data)
int PostUserEvent(LVUserEventRef ref, void* data)
{
return PostLVUserEvent(ref, data);
return PostLVUserEvent(ref, data);
}

//---------------------------------------------------------------------
Expand Down Expand Up @@ -167,7 +167,7 @@ namespace grpc_labview
//---------------------------------------------------------------------
void SetLVString(LStrHandle* lvString, std::string str)
{
auto length = str.length();
auto length = str.length();
auto error = NumericArrayResize(0x01, 1, lvString, length);
memcpy((**lvString)->str, str.c_str(), length);
(**lvString)->cnt = (int)length;
Expand All @@ -176,7 +176,7 @@ namespace grpc_labview
//---------------------------------------------------------------------
//---------------------------------------------------------------------
std::string GetLVString(LStrHandle lvString)
{
{
if (lvString == nullptr || *lvString == nullptr)
{
return std::string();
Expand Down Expand Up @@ -216,4 +216,20 @@ namespace grpc_labview
return clusterOffset;
#endif
}

int32_t GetTypeCodeForSize(int byteSize)
{
switch (byteSize)
{
case 1:
return 0x5; // u8
case 2:
return 0x6; // uW
case 4:
return 0x7; // uL
case 8:
default:
return 0x8; // uQ
}
}
}
13 changes: 8 additions & 5 deletions src/lv_interop.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#define LIBRARY_EXPORT extern "C" __attribute__((visibility("default")))
#endif

namespace grpc_labview
namespace grpc_labview
{
class gRPCid;
extern PointerManager<gRPCid> gPointerManager;
Expand All @@ -50,6 +50,9 @@ namespace grpc_labview

int AlignClusterOffset(int clusterOffset, int alignmentRequirement);

// Provides type code for use with NumericArrayResize function for various sizes of data types.
int32_t GetTypeCodeForSize(int byteSize);

//---------------------------------------------------------------------
// LabVIEW definitions
//---------------------------------------------------------------------
Expand All @@ -64,7 +67,7 @@ namespace grpc_labview
};

using LStrPtr = LStr*;
using LStrHandle = LStr**;
using LStrHandle = LStr**;

struct LV1DArray {
int32_t cnt; /* number of bytes that follow */
Expand Down Expand Up @@ -99,7 +102,7 @@ namespace grpc_labview
#pragma pack (push, 1)
#endif
struct AnyCluster
{
{
LStrHandle TypeUrl;
LV1DArrayHandle Bytes;
};
Expand All @@ -110,11 +113,11 @@ namespace grpc_labview
//---------------------------------------------------------------------
//---------------------------------------------------------------------
void SetLVRTModulePath(std::string modulePath);
void InitCallbacks();
void InitCallbacks();
void SetLVString(LStrHandle* lvString, std::string str);
std::string GetLVString(LStrHandle lvString);
int NumericArrayResize(int32_t typeCode, int32_t numDims, void* handle, size_t size);
int PostUserEvent(LVUserEventRef ref, void *data);
int PostUserEvent(LVUserEventRef ref, void* data);
unsigned char** DSNewHandle(size_t n);
int DSSetHandleSize(void* h, size_t n);
long DSDisposeHandle(void* h);
Expand Down
Loading
Loading