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

nuttx/sched: semaphore and mqueue task list optimize #6987

Merged
merged 2 commits into from Sep 28, 2022

Conversation

zyfeier
Copy link
Contributor

@zyfeier zyfeier commented Sep 2, 2022

Summary

Semaphore and mqueue task list optimize:

  1. move waitingforsemaphore list to sem_s;
  2. move waitingformqnotempty and waitingformqnotfull list to mqueue_inode_s;

depends on #7185

Impact

Code Size:

Before:

   text	   data	    bss	    dec	    hex	filename
 109692	    224	  24940	 134856	  20ec8	nuttx

After:

   text	   data	    bss	    dec	    hex	filename
 109924	    312	  25144	 135380	  210d4	nuttx

Performance:

20 semaphore and 20 threads with different priorities.
sem_post -> sem_wait (cycle count):
before:
8179

after:
6988

Testing

sabre-6quad:nsh

sched/Kconfig Outdated Show resolved Hide resolved
sched/semaphore/sem_post.c Outdated Show resolved Hide resolved
@xiaoxiang781216
Copy link
Contributor

let rebase the patch

@xiaoxiang781216
Copy link
Contributor

@pkarashchenko please review the change where conflict with your priority inheritance change.

@xiaoxiang781216
Copy link
Contributor

@pkarashchenko the break is fixed here apache/nuttx-apps#1321

@pkarashchenko
Copy link
Contributor

@pkarashchenko please review the change where conflict with your priority inheritance change.

This approach goes in conflict with #6318 sched/semaphore/sem_holder.c line 436 that tries to find the task with highest priority that is waiting for the semaphore held by the task. I need some more time to think how to implement fix priority restoration in case we move the list to semaphore

@pkarashchenko
Copy link
Contributor

Yeah, seems it becomes even easier with this change. I just need to pick the first waiter with priority inheritance enabled from each holder list and get the maximum priority among waiters. And execution time will be improved tremendously in such case as we switch search complexity from O(2) to O(1).

@pkarashchenko
Copy link
Contributor

@pkarashchenko the break is fixed here apache/nuttx-apps#1321

@xiaoxiang781216 let's better move include/queue.h to include/nuttx/queue.h

@xiaoxiang781216
Copy link
Contributor

@zyfeier please rebase the patch to the last master.

@zyfeier zyfeier force-pushed the sem_task_list branch 2 times, most recently from 6a339ca to acf4eb2 Compare September 26, 2022 07:15
@xiaoxiang781216
Copy link
Contributor

LGTM.

@masayuki2009
Copy link
Contributor

@xiaoxiang781216 @zyfeier
Let me try this PR later.

@xiaoxiang781216
Copy link
Contributor

Sure.

@masayuki2009
Copy link
Contributor

masayuki2009 commented Sep 27, 2022

@zyfeier
In the PR comment, you wrote

Add CONFIG_TASK_LIST_OPTIMIZE feature to improve sem and mqueue performance.

However, I can not see the config in the commits.

@zyfeier
Copy link
Contributor Author

zyfeier commented Sep 27, 2022

@zyfeier In the PR comment, you wrote

Add CONFIG_TASK_LIST_OPTIMIZE feature to improve sem and mqueue performance.

However, I can not see the config in the commits.

CONFIG_TASK_LIST_OPTIMIZE has been removed,i'll update the PR comment.

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 @zyfeier Let me try this PR later.

@masayuki2009 how about you test result?

@masayuki2009
Copy link
Contributor

@masayuki2009 how about you test result?

@xiaoxiang781216
I confirmed that all the tests passed.

@xiaoxiang781216
Copy link
Contributor

Thanks very much to verify the change.

@xiaoxiang781216 xiaoxiang781216 merged commit 85d013f into apache:master Sep 28, 2022
@zyfeier zyfeier deleted the sem_task_list branch October 10, 2022 06:47
@jerpelea jerpelea added this to To-Add in Release Notes - 12.0.0 Dec 29, 2022
@jerpelea jerpelea moved this from To-Add to Added in Release Notes - 12.0.0 Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants