uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization #14106

dagar · 2020-02-05T20:36:54Z

One of the inefficiencies in legacy orb_subscribe() addressed by the new uORB::Subscription was that attempting to subscribe to a topic would create the underlying device node (the orb topic publication). As a result we had several kilobytes worth of unused orb device nodes in many setups.

In uORB::Subscription we avoid this by checking if the publication actually exists (equivalent to orb_exists()), however each orb_exists() call is relatively expensive. The short term optimization was to throttle the calls within uORB::Subscription after an initial 1000 attempts. The potential problem with this is a subscription might miss initial updates to topics that are only published late in flight.

This PR makes those orb existence checks cheap enough we can use them in every single iteration by adding a bitset of atomic integers to the uORB DeviceMaster. Then I updated the orb generation to also create a class enum for ORB_IDs so that we have an index to use with the bitset.

Master (sensor_accel 0, 1, 2 exist, but 3 and 4 do not)

orb_exists sensor_accel 0: 100 events, 983us elapsed, 9.83us avg, min 9us max 10us 0.377us rms
orb_exists sensor_accel 1: 100 events, 1424us elapsed, 14.24us avg, min 14us max 15us 0.429us rms
orb_exists sensor_accel 2: 100 events, 2031us elapsed, 20.31us avg, min 20us max 21us 0.464us rms
orb_exists sensor_accel 3: 100 events, 3373us elapsed, 33.73us avg, min 33us max 34us 0.446us rms
orb_exists sensor_accel 4: 100 events, 95us elapsed, 0.95us avg, min 1us max 1us 0.219us rms

PR

orb_exists sensor_accel 0: 100 events, 968us elapsed, 9.68us avg, min 9us max 10us 0.468us rms
orb_exists sensor_accel 1: 100 events, 1408us elapsed, 14.08us avg, min 14us max 15us 0.272us rms
orb_exists sensor_accel 2: 100 events, 2027us elapsed, 20.27us avg, min 20us max 21us 0.446us rms
orb_exists sensor_accel 3: 100 events, 161us elapsed, 1.61us avg, min 1us max 2us 0.490us rms
orb_exists sensor_accel 4: 100 events, 95us elapsed, 0.95us avg, min 1us max 1us 0.219us rms
orb_exists sensor_accel 5: 100 events, 89us elapsed, 0.89us avg, min 1us max 1us 0.314us rms
orb_exists sensor_accel 6: 100 events, 81us elapsed, 0.81us avg, min 1us max 1us 0.394us rms
orb_exists sensor_accel 7: 100 events, 90us elapsed, 0.90us avg, min 1us max 1us 0.301us rms
orb_exists sensor_accel 8: 100 events, 90us elapsed, 0.90us avg, min 0us max 1us 0.301us rms
orb_exists sensor_accel 9: 100 events, 91us elapsed, 0.91us avg, min 1us max 1us 0.287us rms
orb_exists sensor_accel 10: 100 events, 91us elapsed, 0.91us avg, min 0us max 1us 0.287us rms

In master orb_exists() for a topic that doesn't exist (with valid instance) is quite expensive because you have to search the entire list of topics. On fmu-v3 configured as a multicopter that's 33.73 us (average). With this PR that same check is 1.61 us. That might seem small, but it adds up across hundreds or even thousands of calls per second.

Overall this saves a little bit of cpu (although quite a lot during the first 5-10 seconds of bootup) and a little bit of memory, despite increasing the size of the orb_metadata struct. More importantly it removes the possibility of subtle unexpected orb update behavior later in flight.

dagar · 2020-02-05T21:03:08Z

@julianoes @bkueng There's a bit more to do here to fix the RTPS build, but could you give this an initial pass? There are cleaner things we could do here if we're willing to drop C support, but I'll save that discussion for another day. I also have some ideas to build off of this for the next round of improvements towards the ultimate goal of uORB type and memory safety.

bkueng · 2020-02-11T19:38:32Z

Looks good from a first look at it - I'll still need to go through the details.

dagar · 2020-02-13T18:30:26Z

TODO (in a followup PR later): store orb field names in a separate structure so that they can be GC'd in stripped down configurations that don't use logger or reply (eg CAN nodes #14148).

dagar · 2020-02-13T20:41:41Z

The bitsets and testing were plucked from #11318.

- add PX4 bitset and atomic_bitset with testing - add uORB::Subscription constructor to take ORB_ID enum - move orb test messages into msg/

bkueng

Did you check the difference in boot time?

msg/templates/uorb/uORBTopics.hpp.em

platforms/common/include/px4_platform_common/atomic_bitset.h

bkueng · 2020-02-14T08:30:47Z

msg/templates/uorb/uORBTopics.hpp.em

+ */
+extern const struct orb_metadata *const *orb_get_topics() __EXPORT;
+
+enum class ORB_ID : uint8_t {


Won't this cause troubles with the ORB_ID macro? At the least it will be confusing.

We can get away with it because of the parentheses, but I'm not sure how questionable it is. The idea is make them equivalent and interchangeable in most uORB usage and possibly transition entirely to the enum class version later.

If we were ready/willing to drop orb C support entirely we could actually get rid of the ORB_ID macro as a wrapper for orb meta pointer (eg __orb_vehicle_status) and replace it with a single byte enum class end-to-end.

Then ORB_ID(vehicle_status) would simply give you ORB_ID::vehicle_status.

src/modules/uORB/uORB.h

src/modules/logger/logged_topics.h

src/modules/uORB/Subscription.hpp

src/modules/uORB/uORBDeviceMaster.cpp

src/modules/uORB/uORBDeviceMaster.hpp

dagar · 2020-02-17T17:56:55Z

Did you check the difference in boot time?

Not accurately, but I wouldn't expect it to be noticeably different. What will be noticeable is the cpu usage is significantly lower immediately after boot compared to current master where it's hammering orb_exists heavily for the first 10-20 seconds.

…instance

dannyfpv · 2020-02-17T21:42:32Z

Tested on Pixhawk 4 v5

Flight Card 1

Modes Tested:
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.

Procedure:
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode.

Notes:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=e5f7fa23-0cc0-49bc-9579-1b8fc35eae76

Flight Card 2

Modes Tested
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

Armed form QGC.
Took-off on mission plan mode
. Note that commands were sent form QGC (Armed and takeoff)
Once all waypoint completed vehicle switched to RTL and landed as expected.
Good flight in general.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=d5245655-61d1-4a77-8120-b701c7f18793

Flight Card 3
Modes Tested
Position Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint
Good flight in general.

Note:
No issues were noted, good flight in general.

Logs:

https://review.px4.io/plot_app?log=2b4c8524-f89c-4555-ad09-148ded67a646

Flight Card 4
Modes Tested
Fail safe:The radio control turned off and the vehicle returned and landed correctly.
Tested good.

Logs:
https://review.px4.io/plot_app?log=5c4d7813-da17-45db-9204-be2c6fdf220b

jorge789 · 2020-02-17T22:50:18Z

Tested on PixRacer V4

Flight Card 1

Modes Tested:
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.

Procedure:
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode.

Notes:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=1266da62-77c8-4240-92e2-f252f816b4a4

Flight Card 2

Modes Tested
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

Armed form QGC.
Took-off on mission plan mode
. Note that commands were sent form QGC (Armed and takeoff)
Once all waypoint completed vehicle switched to RTL and landed as expected.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=e342b6a8-836d-4c27-9429-bd3c0e35ca10

Flight Card 3

Modes Tested
Position Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC,once completed all waypoint then swith RTL.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=249c19a1-aee7-4aad-90b5-c8188bf15de1

Flight Card 4

- Modes Tested
Position Mode: Good.
Failsafe RTL (Return To Land): Not returning after RC shutdown.
- Procedure
Arm and take off in position mode, after flying approximately a few seconds, turned off the RC to activate the fail-safe RTL, wait for the vehicle to return and land.
- Notes
No issue noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=9a17acc3-27d2-419a-aa04-a8824f4e06b8

Tested on CUAV nano V5

Flight Card 1

Modes Tested:
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.

Procedure:
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode.

Notes:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=501bba47-295e-41da-87e6-3d125338b5ff

Flight Card 2

Modes Tested
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

Armed form QGC.
Took-off on mission plan mode
. Note that commands were sent form QGC (Armed and takeoff)
Once all waypoint completed vehicle switched to RTL and landed as expected.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=7a20f475-00d0-4d68-9b37-e62116082a20

Flight Card 3

Modes Tested
Position Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC,once completed all waypoint then swith RTL.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=9387a417-3dce-4a3a-83f9-9354b7f4ee53

Flight Card 4

- Modes Tested
Position Mode: Good.
Failsafe RTL (Return To Land): Not returning after RC shutdown.
- Procedure
Arm and take off in position mode, after flying approximately a few seconds, turned off the RC to activate the fail-safe RTL, wait for the vehicle to return and land.
- Notes
No issue noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=590db8a9-a5ea-491b-a0d3-5b169d487ea0

Tested on NXP V3

Flight Card 1

Modes Tested:
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.

Procedure:
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode.

Notes:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=16dd32ef-2f40-47dc-8b68-516c61d239be

Flight Card 2

Modes Tested
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure

Armed form QGC.
Took-off on mission plan mode
. Note that commands were sent form QGC (Armed and takeoff)
Once all waypoint completed vehicle switched to RTL and landed as expected.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=675540ad-aba5-4f77-b4b2-deca6c5b9606

Flight Card 3

Modes Tested
Position Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC,once completed all waypoint then swith RTL.
Note:
No issues were noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=97d4e29a-ba21-4b11-a476-0f3fb791251c

Flight Card 4

- Modes Tested
Position Mode: Good.
Failsafe RTL (Return To Land): Not returning after RC shutdown.
- Procedure
Arm and take off in position mode, after flying approximately a few seconds, turned off the RC to activate the fail-safe RTL, wait for the vehicle to return and land.
- Notes
No issue noted, good flight in general.

Logs: https://review.px4.io/plot_app?log=36d829a1-da6e-416c-9a82-edd433879ca1

dagar · 2020-03-10T21:22:12Z

Any other feedback?

bkueng

Good to go from my side.

julianoes

Awesome! 🎉

dagar · 2020-03-11T13:05:46Z

Thanks everyone.

mrpollo · 2020-03-13T16:21:00Z

🎉 thanks @dagar

dagar mentioned this pull request Feb 6, 2020

mavlink delete MavlinkOrbSubscription and uORB delete unused orb_stat and last update timestamp #14051

Merged

weekly-digest bot mentioned this pull request Feb 9, 2020

Weekly Digest (2 February, 2020 - 9 February, 2020) #14124

Closed

PX4 deleted a comment from codecov bot Feb 13, 2020

dagar force-pushed the pr-uorb_exists branch from 9ecbf88 to 3a76f2d Compare February 13, 2020 20:39

dagar changed the title ~~[WIP] uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization~~ uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization Feb 13, 2020

dagar marked this pull request as ready for review February 13, 2020 20:39

dagar requested review from bkueng and julianoes February 13, 2020 20:40

dagar added the Admin: Enhancement (improvement) 💡 label Feb 13, 2020

dagar added this to the Release v1.11.0 milestone Feb 13, 2020

uORB ORB_ID enum and rework (fast orb_exists)

1c1930a

- add PX4 bitset and atomic_bitset with testing - add uORB::Subscription constructor to take ORB_ID enum - move orb test messages into msg/

dagar force-pushed the pr-uorb_exists branch from 3a76f2d to 1c1930a Compare February 13, 2020 21:05

dagar requested a review from a team February 13, 2020 21:05

bkueng reviewed Feb 14, 2020

View reviewed changes

weekly-digest bot mentioned this pull request Feb 16, 2020

Weekly Digest (9 February, 2020 - 16 February, 2020) #14167

Closed

dagar added 5 commits February 17, 2020 13:00

Merge branch 'master' into pr-uorb_exists

e861e22

uORBTopics.hpp.em fix indexing

7654185

bitsets rename element_index -> element_mask

c0063fd

uORB::Subscription remove obsolete comment

77f9e98

uORBDeviceMaster deviceNodeExists add checks for ORB_ID::INVALID and …

a9d86a4

…instance

Merge branch 'master' into pr-uorb_exists

5c2a3a9

dagar requested a review from bkueng February 20, 2020 01:36

weekly-digest bot mentioned this pull request Feb 23, 2020

Weekly Digest (16 February, 2020 - 23 February, 2020) #14213

Closed

dagar added 3 commits March 2, 2020 14:17

Merge branch 'master' into pr-uorb_exists

cf82660

Merge remote-tracking branch 'px4/master' into pr-uorb_exists

dc6ea33

bitsets: less verbose count()

d804ee9

bkueng approved these changes Mar 11, 2020

View reviewed changes

julianoes approved these changes Mar 11, 2020

View reviewed changes

dagar merged commit 9585055 into master Mar 11, 2020

dagar deleted the pr-uorb_exists branch March 11, 2020 13:06

TSC21 mentioned this pull request Mar 12, 2020

RTPS: add IDs to 'orb_test' related multitopics #14375

Merged

dagar mentioned this pull request Mar 12, 2020

Update ecl to add ability to recover from bad magnetic yaw #14301

Merged

dagar mentioned this pull request Mar 19, 2020

Logs are unable to be used by the ekf2 replay facility #14422

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization #14106

uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization #14106

dagar commented Feb 5, 2020 •

edited

Loading

dagar commented Feb 5, 2020 •

edited

Loading

bkueng commented Feb 11, 2020

dagar commented Feb 13, 2020

dagar commented Feb 13, 2020

bkueng left a comment

bkueng Feb 14, 2020

dagar Feb 17, 2020

dagar Feb 17, 2020

dagar commented Feb 17, 2020

dannyfpv commented Feb 17, 2020 •

edited by Junkim3DR

Loading

jorge789 commented Feb 17, 2020 •

edited

Loading

dagar commented Mar 10, 2020

bkueng left a comment

julianoes left a comment

dagar commented Mar 11, 2020

mrpollo commented Mar 13, 2020

uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization #14106

uORB add bitset for faster orb_exists check and remove uORB::Subscription lazy subscribe hack/optimization #14106

Conversation

dagar commented Feb 5, 2020 • edited Loading

Master (sensor_accel 0, 1, 2 exist, but 3 and 4 do not)

PR

dagar commented Feb 5, 2020 • edited Loading

bkueng commented Feb 11, 2020

dagar commented Feb 13, 2020

dagar commented Feb 13, 2020

bkueng left a comment

Choose a reason for hiding this comment

bkueng Feb 14, 2020

Choose a reason for hiding this comment

dagar Feb 17, 2020

Choose a reason for hiding this comment

dagar Feb 17, 2020

Choose a reason for hiding this comment

dagar commented Feb 17, 2020

dannyfpv commented Feb 17, 2020 • edited by Junkim3DR Loading

Tested on Pixhawk 4 v5

jorge789 commented Feb 17, 2020 • edited Loading

Tested on PixRacer V4

Tested on CUAV nano V5

Tested on NXP V3

dagar commented Mar 10, 2020

bkueng left a comment

Choose a reason for hiding this comment

julianoes left a comment

Choose a reason for hiding this comment

dagar commented Mar 11, 2020

mrpollo commented Mar 13, 2020

dagar commented Feb 5, 2020 •

edited

Loading

dagar commented Feb 5, 2020 •

edited

Loading

dannyfpv commented Feb 17, 2020 •

edited by Junkim3DR

Loading

jorge789 commented Feb 17, 2020 •

edited

Loading