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

Add parameter to dccl protoc plugin to allow generation of load file for processed protos #106

Merged
merged 6 commits into from
Apr 4, 2023

Conversation

tsaubergine
Copy link
Member

@tsaubergine tsaubergine commented Mar 29, 2023

Overview

The DCCL protoc plugin (protoc-gen-dccl) now optionally accepts a parameter dccl3_load_file that provides a path to the desired output of a .cpp file which will include the required code to load and unload all DCCL messages compiled with this parameter using the existing C-functions dccl3_load and dccl3_unload.

If the file already exists protoc-gen-dccl will not regenerate it, but rather add any new loaders that are missing to the end of the file. This allows the option of this file being part of the (version-controlled) source code and other custom manual actions performed in the dccl3_load and dccl3_unload functions (such as FieldCodecManager add and remove functions).

Closes #103

Usage

Set protoc to use the DCCL plugin (protoc-gen-dccl) and add the dccl3_load_file parameter to the end

BUILD_INCLUDE="/path/to/my/build/include"
PROTO_SRC_BASE="/path/to/my/src/"
PROTOC_GEN_DCCL="/path/to/protoc-gen-dccl"
protoc --cpp_out ${BUILD_INCLUDE} ${PROTO_SRC_BASE}/my.proto --dccl_out=dccl3_load_file=/path/to/my/desired/dccl_load.cpp:${BUILD_INCLUDE} --plugin ${PROTOC_GEN_DCCL}

protoc can be run with multiple protos in a single call or multiple times with the same dccl3_load_file and they will all be appended to the dccl3_load_file given.

At this point you can build a shared library (e.g. libmy_protos.so) as desired with the your my.pb.cc and dccl_load.cpp file and then you can load it with:

dccl::Codec codec;
codec.load_library("libmy_protos.so");

Example Generated File

The dccl3_load_file that is generated by new unit test dccl_load_file:

#include <dccl/codec.h>

// DO NOT REMOVE: required by loader code
std::vector<const google::protobuf::Descriptor*> descriptors;
template<typename PB> struct DCCLLoader { DCCLLoader() { descriptors.push_back(PB::descriptor()); }};
// END: required by loader code


extern "C"
{
    void dccl3_load(dccl::Codec* dccl)
    {
        // DO NOT REMOVE: required by loader code
        for(auto d : descriptors)
            dccl->load(d);
        // END: required by loader code
    }

    void dccl3_unload(dccl::Codec* dccl)
    {
        // DO NOT REMOVE: required by loader code
        for(auto d : descriptors)
            dccl->unload(d);
        // END: required by loader code
    }
}

// BEGIN (TO END OF FILE): AUTOGENERATED LOADERS
#include "dccl/test/dccl_load_file/test2.pb.h"
DCCLLoader<dccl::test::TestMessage3> dccl_test_testmessage3_loader;
#include "dccl/test/dccl_load_file/test1.pb.h"
DCCLLoader<dccl::test::TestMessage1> dccl_test_testmessage1_loader;
DCCLLoader<dccl::test::TestMessage2> dccl_test_testmessage2_loader;

…cpp suitable for generating a shared library that will automatically load all DCCL messages in that library when Codec::load_library is called
@psmskelton
Copy link
Contributor

Thanks for this @tsaubergine! Certainly simplifies some things. However, there are 2 issues that I'm noticing.

Issue 1

I'm missing some DCCLLoader<> entries from each .proto input when there are messages with similar names.

Given the input files:

bits.proto : 50 message definitions
pieces.proto : 11 message definitions

The generated dccl3_load_file file only has:

#include bits.pb.h
DCCLLoader<...> * 49 (missing 1)

#include pieces.pb.h
DCCLLoader<...> * 9 (missing 2)

Manually checking both the CPP and Python output files generated by protoc shows protoc generated the correct number of classes:

# grep -c "@@protoc_insertion_point(class_definition:" bits.pb.h
# 50
# grep -c "_reflection.GeneratedProtocolMessageType("  bits_pb2.py
# 50
# grep -c "@@protoc_insertion_point(class_definition:" pieces.pb.h
# 11
# grep -c "_reflection.GeneratedProtocolMessageType("  pieces_pb2.py
# 11

One message that is persistently not generating a DCCLLoader entry is (grossly simplified):

message Waypoint
{
  option (dccl.msg).codec_version = 4;
  option (dccl.msg).id = 175;
  option (dccl.msg).max_bytes = 2;

  required uint32 testing = 1
  [
    (dccl.field).min = 0,
    (dccl.field).max = 1000000,
    (dccl.field).resolution = 1000
  ];
}

Which is message 47 of 50 in the file (by ID and placement), so it's not EOF problems. Messages that build upon this message, for example:

message WaypointTask
{
  option (dccl.msg).codec_version = 4;
  option (dccl.msg).id = 173;
  option (dccl.msg).max_bytes = 2;

  repeated Waypoint tasks = 1
  [
    (dccl.field).min_repeats = 1,
    (dccl.field).max_repeats = 5
  ];
}

Are compiling fine. If I rename bits.proto ID 175 from Waypoint to Testing, it now generates DCCLLoader<> correctly. The same pattern emerges with pieces.proto, where a message with name VehicleStatus has no DCCLLoader<> entry generated when a message with name VehicleStatusRequest is also present.

Issue 2

It is now difficult to identify non-compliant messages as the exception thrown does not state what descriptor it has a problem with. For example, in a Python development workflow, no further traceback is available beyond:

  File "/usr/local/lib/python3.8/dist-packages/<our_package>/__init__.py", line 18, in get_codec
    codec.load_library("/usr/local/lib/lib<our_library>.so")
dccl.DcclException: Actual maximum size of message exceeds allowed maximum (dccl.max_bytes). Tighten bounds, remove fields, improve codecs, or increase the allowed dccl.max_bytes

Suggested change is for all Exceptions to include the descriptor.full_name in their messages, e.g.:

dccl.DcclException: Actual maximum size of '<descriptor.full_name>' exceeds allowed maximum (dccl.max_bytes). Tighten bounds, remove fields, improve codecs, or increase the allowed dccl.max_bytes

@tsaubergine
Copy link
Member Author

tsaubergine commented Mar 30, 2023

Thanks for your feedback, it's very helpful!

Issue 1: Stupid bug on my part, fixed now (protoc-gen-dccl searches for the message loader to avoid adding it multiple times, but this would falsely pick up messages that were a substring of an existing message. Avoided by adding "<" and ">" to the search string to pick up the complete template)
Issue 2: Good point, I've added it now so Exception messages will look like:

Message: dccl.test.TestMsg: Actual maximum size of message (265B) exceeds allowed maximum (1B). Tighten bounds, remove fields, improve codecs, or increase the allowed value in (dccl.msg).max_bytes

where "Message: dccl.test.TestMsg" will be added whenever the descriptor is known.

#106 introduced exposed unwanted behaviour in the DCCL CLI application. Calling dccl --dlopen /path/to/shared/library.so --analyze dumped the entire library contents to terminal, ignoring the --messages``` flag. This fixes that behaviour.
Fixed DCCL CLI ignoring --messages with --dlopen
@tsaubergine
Copy link
Member Author

@philboske Let me know if this works OK for you now and I'll merge it in (will be part of release 4.1.0)

@psmskelton
Copy link
Contributor

@tsaubergine Seems to be functioning correctly for us. Thanks again.

@tsaubergine tsaubergine merged commit 8e09a56 into 4.0 Apr 4, 2023
@tsaubergine tsaubergine deleted the 4.0-load-file branch April 4, 2023 01:02
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.

Query: Recommended method for deploying a dccl::Codec in .so format?
2 participants