You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/06/08 10:30:36 UTC

[GitHub] [incubator-nuttx] anchao opened a new pull request, #6392: sched/mqueue: remove sched_lock to improve performance

anchao opened a new pull request, #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392

   
   
   ## Summary
   
   sched/mqueue: remove sched_lock to improve performance
   
   remove the sched_lock/unlock to improve the performance by 18%
   
   ```
   mq_send Flow                        Cycle Count
   mq_send                         Origin  Optimized
   |
    ->nxmq_send                       24         24
      |
       ->file_mq_send                209        209
         |
         |->sched_lock               243        N/A  <-
         |->nxmq_do_send             391        348
         |  |
         |  |->sched_lock            434        N/A  <-
         |  |->up_unblock_task       545        459
         |   ->sched_unlock          675        N/A  <-
         |
          ->sched_unlock             684        N/A  <-
            |
             ->up_release_pending    701        N/A
               |
                ->arm_switchcontext  856        610
   
   
   mq_receive
   |
    ->arm_fullcontextrestore        1375       1133
      |
       ->up_block_task              1375       1133
         |
          ->nxmq_wait_receive       1530       1288
            |
             ->file_mq_receive      1606       1310
               |
                ->nxmq_receive      1616       1320
                  |
                   ->mq_receive     1628       1332  - 18%
   ```
   
   
   
   ![Screenshot from 2022-06-08 18-23-42](https://user-images.githubusercontent.com/758493/172594966-c717038c-8e94-42e4-b40f-bca81669e67f.png)
   
   Signed-off-by: chao.an <an...@xiaomi.com>
   
   ## Impact
   message queue
   
   ## Testing
   
   message queue test 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1150897821

   As long as I look into the code, the code seems to be protected by critical sections.
   However, we need more testing.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
anchao commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1149878382

   > 
   
   Yes, I verified this PR on sabre-6quad:smp and custom cortex-a7 dual core device (ostest)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1149862635

   @anchao 
   Did you confirm that this PR works in SMP mode without any side effects?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#discussion_r893245427


##########
sched/mqueue/mq_notify.c:
##########
@@ -114,13 +114,9 @@ int mq_notify(mqd_t mqdes, FAR const struct sigevent *notification)
       /* No.. return EBADF */
 
       errval = EBADF;
-      goto errout_without_lock;
+      goto errout;
     }
 
-  /* Get a pointer to the message queue */
-
-  sched_lock();

Review Comment:
   should we add critical section here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#discussion_r893452094


##########
sched/mqueue/mq_notify.c:
##########
@@ -114,13 +114,9 @@ int mq_notify(mqd_t mqdes, FAR const struct sigevent *notification)
       /* No.. return EBADF */
 
       errval = EBADF;
-      goto errout_without_lock;
+      goto errout;
     }
 
-  /* Get a pointer to the message queue */
-
-  sched_lock();

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
anchao commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1150642300

   > @anchao
   > 
   > You have just pushed several new commits which are not sched_lock-related changes. What is your intention for the commits?
   
   Other patches have some minor code tuning of message queue, mainly to improve the performance further,maybe I should create a new PR to track ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1150638587

   @anchao 
   
   You have just pushed several new commits which are not sched_lock-related changes.
   What is your intention for the commits?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1150691869

   > @anchao Did you confirm that this PR works in SMP mode without any side effects?
   
   sched_lock/sched_unlock can't protect the resource in SMP mode. All code which depend on sched_lock/sched_unlock to protect the resource must be change to the critical section, spinlock or semaphore as soon as we can.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1150643596

   >Other patches have some minor code tuning of message queue, mainly to improve the performance further,maybe I should create a new PR to track ?
   
   @anchao 
   I prefer to create a new PR for it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
anchao commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1150635694

   > Is it tested with multiple threads trying to receive data from same mqueue? I mean the concurrent environment test
   
   Yes, I conducted a local concurrency test on the same mqueue, with 4 threads (2 threads recv / 2 threads send)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] masayuki2009 merged pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
masayuki2009 merged PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6392: sched/mqueue: remove sched_lock to improve performance

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6392:
URL: https://github.com/apache/incubator-nuttx/pull/6392#issuecomment-1149975516

   Is it tested with multiple threads trying to receive data from same mqueue? I mean the concurrent environment test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org