eProsima / Fast-DDS-Gen

Fast-DDS IDL code generator tool. Looking for commercial support? Contact info@eprosima.com
Apache License 2.0
77 stars 58 forks source link

redundant call `register<IDL_MESSAGE_NAME>Types` on msg obj creation #184

Closed fujitatomoya closed 1 year ago

fujitatomoya commented 1 year ago

Is there an already existing issue for this?

Expected behavior

register<IDL_MESSAGE_NAME>Types should be called once for the process space.

the following is pseudo code.

+ #include <mutex>

HelloWorld::HelloWorld()
{
    // unsigned long m_index
    m_index = 0;
    // string m_message
    m_message ="";

    // Just to register all known types
-    registerHelloWorldTypes();
+    static std::once_flag once_flag;
+    std::call_once(once_flag, registerHelloWorldTypes);
}

Current behavior

register<IDL_MESSAGE_NAME>Types is called every single time for message object creation.

e.g)

https://github.com/eProsima/Fast-DDS/blob/23be906a7ec0f0ce3af745540479147e8657d903/examples/cpp/dds/ContentFilteredTopicExample/HelloWorld.cxx#L41-L50

Steps to reproduce

create HelloWorld object in the application.

Fast DDS version/commit

https://github.com/eProsima/Fast-DDS/commit/23be906a7ec0f0ce3af745540479147e8657d903

Platform/Architecture

Ubuntu Focal 20.04 amd64, Ubuntu Focal 20.04 arm64

Transport layer

Default configuration, UDPv4 & SHM

Additional context

it is probably expected that reuse the object so that this problem can be avoided in the application. but this is application burden to limit how to manage the message object in the application layer. besides, this can be avoided in the code generation phase to call register type just once in the process space.

XML configuration file

No response

Relevant log output

No response

Network traffic capture

No response

fujitatomoya commented 1 year ago

CC: @MiguelCompany @iuhilnehc-ynos

fujitatomoya commented 1 year ago

current discussion and assumption

reference to generate the registerXXXTypes here, https://github.com/eProsima/Fast-DDS-Gen/blob/master/src/main/java/com/eprosima/fastcdr/idl/templates/TypeObjectSource.stg#L46

@MiguelCompany @iuhilnehc-ynos

fujitatomoya commented 1 year ago

@iuhilnehc-ynos can you allocate time for this? i believe that we already have the same understanding for https://github.com/eProsima/Fast-DDS-Gen/issues/184 including internal discussion.

MiguelCompany commented 1 year ago

@fujitatomoya @iuhilnehc-ynos I'm moving this into Fast-DDS-Gen. It belongs there

iuhilnehc-ynos commented 1 year ago

They're different updates between

+ #include <mutex>

HelloWorld::HelloWorld()
{
    // unsigned long m_index
    m_index = 0;
    // string m_message
    m_message ="";

    // Just to register all known types
-    registerHelloWorldTypes();
+    static std::once_flag once_flag;
+    std::call_once(once_flag, registerHelloWorldTypes);
}

and

current discussion and assumption

  • std::call_once(once_flag, registerHelloWorldTypes); should be called in registerHelloWorldTypes since which is public method that user application can issue.
  • adding documentation would be ideal to explain this.

reference to generate the registerXXXTypes here, https://github.com/eProsima/Fast-DDS-Gen/blob/master/src/main/java/com/eprosima/fastcdr/idl/templates/TypeObjectSource.stg#L46

The final expected source code will be generated by https://github.com/eProsima/Fast-DDS-Gen/blob/master/src/main/java/com/eprosima/fastcdr/idl/templates/TypeObjectSource.stg#L46.

MiguelCompany commented 1 year ago

Fixed via #185