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

Free the changes that have been acknowledged by all readers (on volatile topics) [19800] #71

Merged
merged 6 commits into from
Nov 8, 2023

Conversation

Tempate
Copy link
Contributor

@Tempate Tempate commented Nov 3, 2023

In the previous version, the changes on reliable-volatile topics were never removed from the RTPS writers' histories. This lead to the congestion of RTPS writers' histories and, at times, to memory exhaustion issues. In this version, RTPS writers free changes (on reliable-volatile topics) once they have been acknowledged by all readers.

In the following example, we launch a DDS Router with two RTPS participants, and publish a hundred 1MB messages on a reliable-volatile topic. Here's the heap memory consumption of the DDS Router before and after this update.

Before (Peak Heap Memory 99.5 MiB):
rtps_rtps

After (Peak Heap Memory 5.9 MiB):
rtps_rtps

This version also includes a bugfix. In the previous version, RTPS CommonWriters would free space in their full-histories after adding a change. This meant that when the depth of the history was set to 1, for example, TRANSIENT-LOCAL wouldn't work, since, after adding the change, the writer would identify its history as full and free space for another change (deleting the previous one). Now, however, freeing space in full histories is done before adding a change, which fixes the problem.

@Tempate Tempate changed the title Remove from the writers' history the changes that have been acknowledged by all readers (on reliable-volatile topics) [19800] Free the changes that have been acknowledged by all readers (on volatile topics) [19800] Nov 3, 2023
@Tempate Tempate force-pushed the feature/reduce_memory_usage branch from 5bffc1b to 4a0f594 Compare November 7, 2023 15:48
Copy link
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment

Choose a reason for hiding this comment

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

LGTM

@juanlofer-eprosima juanlofer-eprosima merged commit 6fa64b8 into main Nov 8, 2023
12 checks passed
@juanlofer-eprosima juanlofer-eprosima deleted the feature/reduce_memory_usage branch November 8, 2023 14:57
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.

2 participants