Multithread improvements by pablogs9 · Pull Request #205 · micro-ROS/rmw_microxrcedds

pablogs9 · 2021-11-24T11:54:54Z

Required: eProsima/Micro-XRCE-DDS-Client#289

github-actions · 2021-11-24T12:06:24Z

Static memory analysis

Default configuration

MTU: 512 B
Input buffer size: 2048 B
Input history: 4
Output buffer size: 2048 B
Output history: 4

Entity	Qty	Size per unit
Context	2	5616 B
Topic	8	56 B
Service	4	248 B
Client	4	248 B
Subscription	4	272 B
Publisher	4	288 B
Node	4	208 B
Static input buffer	8	2136 B
Init options	6	64 B
Wait sets	4	56 B
Guard Condition	4	64 B

TOTAL: 34468 B

Acuadros95 · 2021-11-24T12:10:38Z


+  // With `rmw_uxrce_wait_mutex` member `need_to_be_ran` is protected.
+  // `session_memory` itself is not protected because it is not modified between
+  // rmw_init and rmw_shutdown, and rmw_wait cannot be called concurrently with


Maybe we should add a multi-thread section to Programming with rcl and rclc or XRCE-Client Documentation to add this guidelines.

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>

github-actions · 2021-11-24T12:40:02Z

Static memory analysis

Default configuration

MTU: 512 B
Input buffer size: 2048 B
Input history: 4
Output buffer size: 2048 B
Output history: 4

Entity	Qty	Size per unit
Context	2	5616 B
Topic	8	56 B
Service	4	248 B
Client	4	248 B
Subscription	4	272 B
Publisher	4	288 B
Node	4	208 B
Static input buffer	8	2136 B
Init options	6	64 B
Wait sets	4	56 B
Guard Condition	4	64 B

TOTAL: 34468 B

github-actions · 2021-11-24T12:41:24Z

Static memory analysis

Default configuration

MTU: 512 B
Input buffer size: 2048 B
Input history: 4
Output buffer size: 2048 B
Output history: 4

Entity	Qty	Size per unit
Context	2	5616 B
Topic	8	56 B
Service	4	248 B
Client	4	248 B
Subscription	4	272 B
Publisher	4	288 B
Node	4	208 B
Static input buffer	8	2136 B
Init options	6	64 B
Wait sets	4	56 B
Guard Condition	4	64 B

TOTAL: 34468 B

Acuadros95 · 2021-11-24T12:41:26Z

+      success = uxr_ping_agent_session(&context->session, timeout_ms, attempts);
+
      item = item->next;
    } while (NULL != item && !success);


I still dont like the ping multiple sessions approach.

Maybe we could change this to accept a rmw_context_impl_t as argument? Wich could be NULL.

Don't understand, can you make a proposal?

…rmw-microxrcedds into fix/lock_wait_session

github-actions · 2021-11-24T12:52:52Z

Static memory analysis

Default configuration

MTU: 512 B
Input buffer size: 2048 B
Input history: 4
Output buffer size: 2048 B
Output history: 4

Entity	Qty	Size per unit
Context	2	5616 B
Topic	8	56 B
Service	4	248 B
Client	4	248 B
Subscription	4	272 B
Publisher	4	288 B
Node	4	208 B
Static input buffer	8	2136 B
Init options	6	64 B
Wait sets	4	56 B
Guard Condition	4	64 B

TOTAL: 34468 B

github-actions · 2021-11-25T15:41:16Z

Static memory analysis

Default configuration

MTU: 512 B
Input buffer size: 2048 B
Input history: 4
Output buffer size: 2048 B
Output history: 4

Entity	Qty	Size per unit
Context	2	5616 B
Topic	8	56 B
Service	4	248 B
Client	4	248 B
Subscription	4	272 B
Publisher	4	288 B
Node	4	208 B
Static input buffer	8	2136 B
Init options	6	64 B
Wait sets	4	56 B
Guard Condition	4	64 B

TOTAL: 34468 B

pablogs9 · 2021-11-25T15:41:53Z

@mergify backport foxy main

* Minor fixes * Add rmw_wait mutex * Protect static_buffer_memory access * Use new ping API * Update ci.yml * Fix * Fix linter * Apply suggestions from code review Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com> * Fix * Delete old unlock mutex * Update .github/workflows/ci.yml Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com> Co-authored-by: Antonio Cuadros <acuadros1995@gmail.com> (cherry picked from commit 5de3d76) # Conflicts: # .github/workflows/ci.yml

mergify · 2021-11-25T15:42:36Z

backport foxy main

✅ Backports have been created

Details

#206 Multithread improvements (backport #205) has been created for branch foxy
#207 Multithread improvements (backport #205) has been created for branch main

Hey, I reacted but my real name is @Mergifyio

* Multithread improvements (#205) * Minor fixes * Add rmw_wait mutex * Protect static_buffer_memory access * Use new ping API * Update ci.yml * Fix * Fix linter * Apply suggestions from code review Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com> * Fix * Delete old unlock mutex * Update .github/workflows/ci.yml Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com> Co-authored-by: Antonio Cuadros <acuadros1995@gmail.com> (cherry picked from commit 5de3d76) # Conflicts: # .github/workflows/ci.yml * Resolve conflicts Signed-off-by: Pablo Garrido <pablogs9@gmail.com> Co-authored-by: Pablo Garrido <pablogs9@gmail.com>

pablogs9 added 4 commits November 24, 2021 12:51

Minor fixes

70eb6d5

Add rmw_wait mutex

d84ef15

Protect static_buffer_memory access

8ef1f82

Use new ping API

682a381

pablogs9 requested a review from Acuadros95 November 24, 2021 11:54

Update ci.yml

c763cae

pablogs9 commented Nov 24, 2021

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

pablogs9 added 2 commits November 24, 2021 12:59

Fix

5171154

Fix linter

660bd5a

Acuadros95 reviewed Nov 24, 2021

View reviewed changes

Comment thread rmw_microxrcedds_c/src/callbacks.c Outdated

Acuadros95 reviewed Nov 24, 2021

View reviewed changes

Comment thread rmw_microxrcedds_c/src/callbacks.c

Acuadros95 reviewed Nov 24, 2021

View reviewed changes

Comment thread rmw_microxrcedds_c/src/callbacks.c Outdated

pablogs9 and others added 2 commits November 24, 2021 13:38

Apply suggestions from code review

a061000

Co-authored-by: Antonio Cuadros <49162117+Acuadros95@users.noreply.github.com>

Fix

5c29194

Acuadros95 reviewed Nov 24, 2021

View reviewed changes

Acuadros95 approved these changes Nov 24, 2021

View reviewed changes

Acuadros95 added 2 commits November 24, 2021 13:50

Delete old unlock mutex

f18c609

Merge branch 'fix/lock_wait_session' of https://github.com/micro-ROS/…

dd9e0ea

…rmw-microxrcedds into fix/lock_wait_session

pablogs9 mentioned this pull request Nov 24, 2021

rmw_uros_ping_agent can cause exception when micro-ROS has not been initialized (MULTITHREAD Feature on) #203

Closed

Update .github/workflows/ci.yml

81615ce

pablogs9 merged commit 5de3d76 into galactic Nov 25, 2021

pablogs9 deleted the fix/lock_wait_session branch November 25, 2021 15:41

mergify Bot mentioned this pull request Nov 25, 2021

Multithread improvements (backport #205) #206

Merged

mergify Bot mentioned this pull request Nov 25, 2021

Multithread improvements (backport #205) #207

Merged

Conversation

pablogs9 commented Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 24, 2021

Static memory analysis

Uh oh!

Acuadros95 Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

pablogs9 Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 24, 2021

Static memory analysis

Uh oh!

github-actions Bot commented Nov 24, 2021

Static memory analysis

Uh oh!

Acuadros95 Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

pablogs9 Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 24, 2021

Static memory analysis

Uh oh!

github-actions Bot commented Nov 25, 2021

Static memory analysis

Uh oh!

pablogs9 commented Nov 25, 2021

Uh oh!

mergify Bot commented Nov 25, 2021

✅ Backports have been created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pablogs9 commented Nov 24, 2021 •

edited

Loading