[23504] Optimize (de)serialization of flat, non-primitive arrays by Mario-DL · Pull Request #274 · eProsima/Fast-CDR

Mario-DL · 2025-07-23T12:57:19Z

Description

This PR makes Fast CDR look for external (de)serialize_array template specializations (now provided by the generated types) so that flat, non-primitive arrays or vectors can be optimally serialized (memcpy) if certain conditions are met.
These changes break API so must be in a new major release.

Related PRs:

[23504] Optimize (de)serialization of flat, non-primitive arrays Fast-DDS-Gen#489
[23504] Add types for the new (de)serialization methods of flat, non-primitive arrays dds-types-test#44
[23504] Optimize (de)serialization of flat, non-primitive arrays Fast-DDS#5953

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
Any new/modified methods have been properly documented using Doxygen.
NO Changes are backport compatible: they do NOT break ABI nor change library core behavior.
NO Changes are API compatible.
New feature has been added to the versions.md file (if applicable).
N/A Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: CI pass and failing tests are unrelated with the changes.

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

…TypesShortStruct Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

MiguelCompany

Very nice refactor!
I really like the traits and SFINAE being used.

Leaving some comments

MiguelCompany · 2025-07-30T08:31:17Z

test/cdr/container_introspection.cpp

+    static_assert(static_is_multi_array_primitive<uint8_t const*>::value, "uint8_t* should be a multi primitive array");
+    static_assert(static_is_multi_array_primitive<uint16_t const*>::value,
+            "uint16_t* should be a multi primitive array");
+    static_assert(static_is_multi_array_primitive<uint32_t const*>::value,
+            "uint32_t* should be a multi primitive array");
+    static_assert(static_is_multi_array_primitive<uint64_t const*>::value,
+            "uint64_t* should be a multi primitive array");


Missing signed versions of these tests

Also missing tests for enum

MiguelCompany · 2025-07-30T08:43:59Z

test/cdr/container_introspection.cpp

+    static_assert(!static_is_multi_array_primitive<std::string const*>::value,
+            "std::string* should not be a multi primitive array");


Missing fixed_string

MiguelCompany · 2025-07-30T08:46:23Z

include/fastcdr/Cdr.h

+template<class _T>
+extern void serialize_array(
+        Cdr&,
+        const _T*,
+        const size_t);
+
+template<class _T>
+extern void deserialize_array(
+        Cdr&,
+        _T*,
+        const size_t);
+


Add comments for all four declarations

Also change the declaration order. Either serialize, deserialize, serialize_array, deserialize_array or serialize, serialize_array, deserialize, deserialize_array

MiguelCompany · 2025-07-30T08:55:20Z

include/fastcdr/Cdr.h

    Cdr& serialize(
            const std::array<_T, _Size>& array_t)
    {
-        if (!is_multi_array_primitive(&array_t))


Seems is_multi_array_primitive is not used anymore (since we now rely on static_is_multi_array_primitive).
Might be a good idea to remove file container_recursive_inspector.hpp and perhaps also rename static_is_multi_array_primitive into is_multi_array_primitive.

MiguelCompany · 2025-07-30T09:03:32Z

include/fastcdr/Cdr.h

+        Cdr::state dheader_state {allocate_xcdrv2_dheader()};

-            serialize_array(array_t.data(), array_t.size());
+        serialize(static_cast<int32_t>(vector_t.size()));


Suggested change

serialize(static_cast<int32_t>(vector_t.size()));

serialize(static_cast<uint32_t>(vector_t.size()));

We should check that the size is not bigger than the maximum value of a uint32_t.
I propose having a serialize_collection_length(size_t length) method that throws an exception when length > MAX_UINT32, and then serializes static_cast<uint32_t>(length).

Note: this would imply changes in other methods unrelated to this PR. It might be a good idea to do that refactoring on a separate PR that we can backport.

MiguelCompany · 2025-07-30T09:21:25Z

include/fastcdr/Cdr.h

+            if ((end_ - offset_) < sequence_length)
+            {
+                set_state(state_before_error);
+                throw exception::NotEnoughMemoryException(
+                          exception::NotEnoughMemoryException::NOT_ENOUGH_MEMORY_MESSAGE_DEFAULT);
+            }
+
+            try
+            {
+                vector_t.resize(sequence_length);
+                eprosima::fastcdr::deserialize_array(*this, vector_t.data(), vector_t.size());
+            }
+            catch (exception::Exception& ex)
+            {
+                set_state(state_before_error);
+                ex.raise();
+            }


We shall follow this same part in the XCDRv2 part of the if above.

Again, this seems to come from the old code, so a separate PR fixing it that can be backported should be done.

Refs #23504: Bump project major to 3

5944145

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Mario-DL added this to the v3.0.0 milestone Jul 23, 2025

Mario-DL added 5 commits July 23, 2025 15:06

Refs #23504: Add container_introspection_helpers.hpp

ef3491d

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Refs #23504: Add tests for container introspection meta-functions

f8f8dab

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Refs #23504: external (de)serialize_array() for std::array

343f19c

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Refs #23504: external (de)serialize_array() for std::vector

b9baed5

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Refs #23504: Fix tests, implement (de)serialize_array() on InnerBasic…

699810b

…TypesShortStruct Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Mario-DL force-pushed the feature/23504 branch 2 times, most recently from c1a125a to 8a54298 Compare July 23, 2025 13:22

Mario-DL mentioned this pull request Jul 23, 2025

[23504] Optimize (de)serialization of flat, non-primitive arrays eProsima/Fast-DDS-Gen#489

Open

7 tasks

Refs #23504: Linter

931c8dc

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Mario-DL force-pushed the feature/23504 branch from 8a54298 to 931c8dc Compare July 23, 2025 13:43

This was referenced Jul 23, 2025

[23504] Add types for the new (de)serialization methods of flat, non-primitive arrays eProsima/dds-types-test#44

Open

[23504] Optimize (de)serialization of flat, non-primitive arrays eProsima/Fast-DDS#5953

Open

MiguelCompany added versions-pending blocked-till-branch-out labels Jul 30, 2025

MiguelCompany requested changes Jul 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[23504] Optimize (de)serialization of flat, non-primitive arrays#274

[23504] Optimize (de)serialization of flat, non-primitive arrays#274
Mario-DL wants to merge 7 commits intomasterfrom
feature/23504

Mario-DL commented Jul 23, 2025 •

edited by MiguelCompany

Loading

Uh oh!

MiguelCompany left a comment

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

MiguelCompany Jul 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		static_assert(!static_is_multi_array_primitive<std::string const*>::value,
		"std::string* should not be a multi primitive array");

	serialize(static_cast<int32_t>(vector_t.size()));
	serialize(static_cast<uint32_t>(vector_t.size()));

Conversation

Mario-DL commented Jul 23, 2025 • edited by MiguelCompany Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Checklist

Reviewer Checklist

Uh oh!

MiguelCompany left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mario-DL commented Jul 23, 2025 •

edited by MiguelCompany

Loading