introduce IntrusiveSortedList and use for uORB, WorkQueues, and WorkItems #14885
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julianoes
reviewed
Jul 2, 2020
update: 1s, num topics: 64
TOPIC NAME INST #SUB RATE #LOST #Q SIZE
actuator_armed 0 6 1 0 1 24
actuator_controls_0 0 6 998 2793 1 48
actuator_outputs 0 3 800 1914 1 80
adc_report 0 1 100 0 1 96
battery_status 0 6 100 247 1 112
battery_status 1 6 100 247 1 112
commander_state 0 1 1 0 1 16
cpuload 0 4 2 1 1 16
estimator_innovation_test_ratios 0 1 100 0 1 144
estimator_innovation_variances 0 1 100 0 1 144
estimator_innovations 0 1 100 0 1 144
estimator_sensor_bias 0 4 100 93 1 56
estimator_status 0 4 100 367 1 288
multirotor_motor_limits 0 2 800 0 1 16
rate_ctrl_status 0 1 998 0 1 24
safety 0 2 1 0 1 16
sensor_accel 0 4 803 0 8 48
sensor_accel 1 3 750 0 8 48
sensor_baro 0 2 73 0 1 32
sensor_baro 1 2 73 14 1 32
sensor_combined 0 3 200 189 1 48
sensor_gyro 0 4 998 0 8 40
sensor_gyro 1 3 750 0 8 40
sensor_mag 0 3 98 92 1 48
sensor_preflight 0 1 200 0 1 24
system_power 0 2 100 9 1 24
telemetry_status 0 2 2 0 3 48
vehicle_acceleration 0 1 201 101 1 32
vehicle_air_data 0 8 18 90 1 40
vehicle_angular_acceleration 0 1 998 0 1 32
vehicle_angular_velocity 0 5 998 2889 1 32
vehicle_attitude 0 7 200 821 1 48
vehicle_attitude_setpoint 0 4 200 476 1 64
vehicle_control_mode 0 8 1 0 1 32
vehicle_imu 0 3 200 191 1 56
vehicle_imu 1 3 188 179 1 56
vehicle_imu_status 0 4 10 0 1 56
vehicle_imu_status 1 4 10 0 1 56
vehicle_land_detected 0 9 1 5 1 24
vehicle_local_position 0 11 100 820 1 152
vehicle_magnetometer 0 3 97 91 1 24
vehicle_odometry 0 1 100 118 1 256
vehicle_rates_setpoint 0 4 200 476 1 32
vehicle_status 0 17 1 5 1 56
vehicle_status_flags 0 1 1 0 1 40 nsh> work_queue status
Work Queue: 9 threads RATE INTERVAL
|__ 1) wq:rate_ctrl
| |__ 1) mc_rate_control 997.8 Hz 1002 us
| |__ 2) pwm_out 799.6 Hz 1251 us
| \__ 3) vehicle_angular_velocity 997.8 Hz 1002 us
|__ 2) wq:SPI2
| \__ 1) rm3100 97.3 Hz 10282 us
|__ 3) wq:SPI4
| |__ 1) bmi088 997.2 Hz 1003 us
| |__ 2) bmi088 803.0 Hz 1245 us
| \__ 3) ms5611 97.3 Hz 10280 us
|__ 4) wq:SPI6
| |__ 1) icm20649 747.4 Hz 1338 us (1333 us)
| \__ 2) ms5611 97.3 Hz 10281 us
|__ 5) wq:attitude_ctrl
| \__ 1) mc_att_control 199.6 Hz 5011 us
|__ 6) wq:nav_and_controllers
| |__ 1) ekf2 199.6 Hz 5011 us
| |__ 2) land_detector 86.3 Hz 11587 us
| |__ 3) mc_pos_control 50.0 Hz 19991 us
| |__ 4) sensors 199.4 Hz 5015 us
| |__ 5) vehicle_acceleration 215.3 Hz 4645 us
| |__ 6) vehicle_air_data 72.8 Hz 13731 us
| |__ 7) vehicle_imu 187.6 Hz 5332 us
| \__ 8) vehicle_imu 199.8 Hz 5005 us
|__ 7) wq:hp_default
| |__ 1) adc 100.0 Hz 9999 us (10000 us)
| |__ 2) battery_status 100.0 Hz 9998 us (10000 us)
| |__ 3) rc_update 0.0 Hz 0 us
| |__ 4) safety_button 30.3 Hz 32982 us (33000 us)
| \__ 5) tone_alarm 10.3 Hz 96623 us
|__ 8) wq:UART5
| \__ 1) rc_input 250.0 Hz 4000 us (4000 us)
\__ 9) wq:lp_default
|__ 1) load_mon 2.0 Hz 499053 us (500000 us)
|__ 2) mc_hover_thrust_estimator 0.0 Hz 0 us
|__ 3) rgbled_pwm 39.2 Hz 25491 us
\__ 4) send_event 30.0 Hz 33329 us (33333 us)
|
julianoes
approved these changes
Aug 4, 2020
| @@ -57,6 +60,9 @@ class WorkItem : public ListNode<WorkItem *>, public IntrusiveQueueNode<WorkItem | |||
| WorkItem(WorkItem &&) = delete; | |||
| WorkItem &operator=(WorkItem &&) = delete; | |||
|
|
|||
| // WorkItems sorted by name | |||
| bool operator<=(const WorkItem &rhs) const { return (strcmp(ItemName(), rhs.ItemName()) <= 0); } | |||
There was a problem hiding this comment.
Should this have an upper bound for safety, so with strncmp?
There was a problem hiding this comment.
I suppose, although in a lot of these cases for us I think we'd want to go back to the source to get a reasonable answer in place for the max length.
In this case the WorkItem name is set in the constructor and I believe we're currently always setting it to MODULE_NAME or <single_shot>. MODULE_NAME doesn't really have any constraints, although in the past we've trimmed them to fit within the NuttX task CONFIG_TASK_NAME_SIZE.
I'm not sure if we need to worry about about it unless there's the possibility of involving a non-NULL terminated character array.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a new linked list container (c++ template) that sorts on insertion. It's primarily for convenience for inspecting things in the system like uORB or WorkQueues. These are lists of things that are effectively static for the duration of operation in any normal usage (once booted), so there's no issue with doing slightly more work on insertion.
Usage is nearly identical to the existing
Listcontainer other than having to additionally implementoperator<=in yourIntrusiveSortedListNode.