Skip to content

Commit

Permalink
Improve GUID_t operator< performance (#2986)
Browse files Browse the repository at this point in the history 
* Reimplement Guid comparator

Signed-off-by: jparisu <javierparis@eprosima.com>

* Add tests

Signed-off-by: jparisu <javierparis@eprosima.com>

* apply suggestions

Signed-off-by: jparisu <javierparis@eprosima.com>

* Add tests to == and =!

Signed-off-by: jparisu <javierparis@eprosima.com>

* uncrustify

Signed-off-by: jparisu <javierparis@eprosima.com>

Signed-off-by: jparisu <javierparis@eprosima.com>
(cherry picked from commit c871a29)

Co-authored-by: jparisu <69341543+jparisu@users.noreply.github.com>
  • Loading branch information
@mergify @jparisu
mergify[bot] and jparisu authored Oct 5, 2022
1 parent 03fdb06 commit 4b77c39
Show file tree
Hide file tree
Showing 7 changed files with 674 additions and 39 deletions.
52 changes: 34 additions & 18 deletions include/fastdds/rtps/common/EntityId_t.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,40 @@ struct RTPS_DllAPI EntityId_t
return 0x2u & to_uint32() && !is_reader();
}

/**
* Entity Id minor operator
* @param other Second entity id to compare
* @return True if \c other is higher than this
*/
bool operator <(
const EntityId_t& other) const
{
return std::memcmp(value, other.value, size) < 0;
}

/**
* Entity Id compare static method.
*
* @param entity1 First entity id to compare
* @param entity2 Second entity id to compare
*
* @return 0 if \c entity1 is equal to \c entity2 .
* @return < 0 if \c entity1 is lower than \c entity2 .
* @return > 0 if \c entity1 is higher than \c entity2 .
*/
static int cmp(
const EntityId_t& entity1,
const EntityId_t& entity2)
{
return std::memcmp(entity1.value, entity2.value, size);
}

};

#ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC

/**
* Guid prefix comparison operator
* Entity Id comparison operator
* @param id1 EntityId to compare
* @param id2 ID prefix to compare
* @return True if equal
Expand All @@ -214,7 +242,7 @@ inline bool operator ==(
}

/**
* Guid prefix comparison operator
* Entity Id comparison operator
* @param id1 First EntityId to compare
* @param id2 Second EntityId to compare
* @return True if equal
Expand All @@ -223,14 +251,7 @@ inline bool operator ==(
const EntityId_t& id1,
const EntityId_t& id2)
{
for (uint8_t i = 0; i < 4; ++i)
{
if (id1.value[i] != id2.value[i])
{
return false;
}
}
return true;
return EntityId_t::cmp(id1, id2) == 0;
}

/**
Expand All @@ -243,14 +264,9 @@ inline bool operator !=(
const EntityId_t& id1,
const EntityId_t& id2)
{
for (uint8_t i = 0; i < 4; ++i)
{
if (id1.value[i] != id2.value[i])
{
return true;
}
}
return false;
// Use == operator as it is faster enough.
// NOTE: this could be done comparing the entities backwards (starting in [3]) as it would probably be faster.
return !(operator ==(id1, id2));
}

#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
Expand Down
29 changes: 9 additions & 20 deletions include/fastdds/rtps/common/Guid.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ struct RTPS_DllAPI GUID_t
{
return *reinterpret_cast<const InstanceHandle_t*>(this);
}

};

#ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
Expand Down Expand Up @@ -170,29 +169,19 @@ inline bool operator <(
const GUID_t& g1,
const GUID_t& g2)
{
for (uint8_t i = 0; i < 12; ++i)
auto prefix_cmp = GuidPrefix_t::cmp(g1.guidPrefix, g2.guidPrefix);
if (prefix_cmp < 0)
{
if (g1.guidPrefix.value[i] < g2.guidPrefix.value[i])
{
return true;
}
else if (g1.guidPrefix.value[i] > g2.guidPrefix.value[i])
{
return false;
}
return true;
}
for (uint8_t i = 0; i < 4; ++i)
else if (prefix_cmp > 0)
{
if (g1.entityId.value[i] < g2.entityId.value[i])
{
return true;
}
else if (g1.entityId.value[i] > g2.entityId.value[i])
{
return false;
}
return false;
}
else
{
return g1.entityId < g2.entityId;
}
return false;
}

#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
Expand Down
19 changes: 18 additions & 1 deletion include/fastdds/rtps/common/GuidPrefix_t.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,31 @@ struct RTPS_DllAPI GuidPrefix_t
/**
* Guid prefix minor operator
* @param prefix Second guid prefix to compare
* @return True if prefix is higher
* @return True if prefix is higher than this
*/
bool operator <(
const GuidPrefix_t& prefix) const
{
return std::memcmp(value, prefix.value, size) < 0;
}

/**
* Guid Prefix compare static method.
*
* @param prefix1 First guid prefix to compare
* @param prefix2 Second guid prefix to compare
*
* @return 0 if \c prefix1 is equal to \c prefix2 .
* @return < 0 if \c prefix1 is lower than \c prefix2 .
* @return > 0 if \c prefix1 is higher than \c prefix2 .
*/
static int cmp(
const GuidPrefix_t& prefix1,
const GuidPrefix_t& prefix2)
{
return std::memcmp(prefix1.value, prefix2.value, size);
}

#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
};

Expand Down
51 changes: 51 additions & 0 deletions test/unittest/rtps/common/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,54 @@ target_include_directories(PortParametersTests PRIVATE
${PROJECT_SOURCE_DIR}/include ${PROJECT_BINARY_DIR}/include)
target_link_libraries(PortParametersTests GTest::gtest)
add_gtest(PortParametersTests SOURCES ${PORTPARAMETERSTESTS_SOURCE} LABELS "NoMemoryCheck")

###################
# GuifPrefix test #
###################

set(GUIDPREFIXTESTS_SOURCE GuidPrefixTests.cpp)

add_executable(GuidPrefixTests ${GUIDPREFIXTESTS_SOURCE})
target_compile_definitions(GuidPrefixTests PRIVATE FASTRTPS_NO_LIB
$<$<AND:$<NOT:$<BOOL:${WIN32}>>,$<STREQUAL:"${CMAKE_BUILD_TYPE}","Debug">>:__DEBUG>
$<$<BOOL:${INTERNAL_DEBUG}>:__INTERNALDEBUG> # Internal debug activated.
)
target_include_directories(GuidPrefixTests PRIVATE
${PROJECT_SOURCE_DIR}/include ${PROJECT_BINARY_DIR}/include
${PROJECT_SOURCE_DIR}/src/cpp)
target_link_libraries(GuidPrefixTests GTest::gtest)
add_gtest(GuidPrefixTests SOURCES ${GUIDPREFIXTESTS_SOURCE} LABELS "NoMemoryCheck")

#################
# EntityId test #
#################

set(ENTITYIDTESTS_SOURCE EntityIdTests.cpp)

add_executable(EntityIdTests ${ENTITYIDTESTS_SOURCE})
target_compile_definitions(EntityIdTests PRIVATE FASTRTPS_NO_LIB
$<$<AND:$<NOT:$<BOOL:${WIN32}>>,$<STREQUAL:"${CMAKE_BUILD_TYPE}","Debug">>:__DEBUG>
$<$<BOOL:${INTERNAL_DEBUG}>:__INTERNALDEBUG> # Internal debug activated.
)
target_include_directories(EntityIdTests PRIVATE
${PROJECT_SOURCE_DIR}/include ${PROJECT_BINARY_DIR}/include
${PROJECT_SOURCE_DIR}/src/cpp)
target_link_libraries(EntityIdTests GTest::gtest)
add_gtest(EntityIdTests SOURCES ${ENTITYIDTESTS_SOURCE} LABELS "NoMemoryCheck")

#############
# Guid test #
#############

set(GUIDTESTS_SOURCE GuidTests.cpp)

add_executable(GuidTests ${GUIDTESTS_SOURCE})
target_compile_definitions(GuidTests PRIVATE FASTRTPS_NO_LIB
$<$<AND:$<NOT:$<BOOL:${WIN32}>>,$<STREQUAL:"${CMAKE_BUILD_TYPE}","Debug">>:__DEBUG>
$<$<BOOL:${INTERNAL_DEBUG}>:__INTERNALDEBUG> # Internal debug activated.
)
target_include_directories(GuidTests PRIVATE
${PROJECT_SOURCE_DIR}/include ${PROJECT_BINARY_DIR}/include
${PROJECT_SOURCE_DIR}/src/cpp)
target_link_libraries(GuidTests GTest::gtest)
add_gtest(GuidTests SOURCES ${GUIDTESTS_SOURCE} LABELS "NoMemoryCheck")
Loading

0 comments on commit 4b77c39

Please sign in to comment.