| Summary: | ASTERISK-16456: [patch] ast_sched_runq runs to much events if one event runs too long | ||
| Reporter: | Stefan Schmidt (schmidts) | Labels: | |
| Date Opened: | 2010-07-28 16:57:17 | Date Closed: | 2010-08-10 13:05:39 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Core/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) sched.c.patch | |
| Description: | same in 1.8.0 after debugging and tracing the new heap schedule concept i´ve found something iam not sure about if this works like expacted. in sched.c in the ast_sched_runq there is a for loop which takes the next event from the heap and calculate the time which is 1 ms in future make a time compare if this event is between now and the calculated time. if not it breaks and get back to do_monitor. If it founds an event it start the callback and release this event then take the next event and does the time calc and compare again. if the callback of the curent event tooks to much time, or there are too many events the time compare could get events which allready 1 ms further away than expacted. they have been 2 ms or more away at the start time of runq. ****** ADDITIONAL INFORMATION ****** my patch just moves the time calculate before the for loop so only events which are 1 ms away at starting the loop will be runned and not events which will come closer in time during processing other events. what happens without the patch. i´ve seen in my debug tests that runq had stared more than 1500 events in a single run, cause one of them took too long to execute and others was close in time. runq should never needs more than 1 ms, because in this time when runq execute events, the inbound packages are only buffered and not processed. this happens in ast_io_wait which is started before ast_sched_runq while waiting for the next event. i think asterisk performs better, when runq just runs for the time slot of 1 ms from the start time, get back to do_monitor, checks the wait_time for the next event (which may be 0), process incoming packages and than do runq again. i think of a situation where many peers answers keep alive packages, while a big amount of events like pkg_retrans is sent, these inbound packages are ignored until runq has finished. maybe some of these inbound packages would have dropped an event from the heap cause its an answer package. | ||
| Comments: | By: Digium Subversion (svnbot) 2010-08-10 13:04:31 Repository: asterisk Revision: 281574 U branches/1.6.2/main/sched.c ------------------------------------------------------------------------ r281574 | russell | 2010-08-10 13:04:31 -0500 (Tue, 10 Aug 2010) | 9 lines Don't move the time threshold for running scheduled events on every iteration. Instead, only calculate the time threshold each time ast_sched_runq() is called. (closes issue ASTERISK-16456) Reported by: schmidts Patches: sched.c.patch uploaded by schmidts (license 1077) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=281574 By: Digium Subversion (svnbot) 2010-08-10 13:05:06 Repository: asterisk Revision: 281575 _U branches/1.8/ U branches/1.8/main/sched.c ------------------------------------------------------------------------ r281575 | russell | 2010-08-10 13:05:05 -0500 (Tue, 10 Aug 2010) | 16 lines Merged revisions 281574 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........ r281574 | russell | 2010-08-10 13:04:32 -0500 (Tue, 10 Aug 2010) | 9 lines Don't move the time threshold for running scheduled events on every iteration. Instead, only calculate the time threshold each time ast_sched_runq() is called. (closes issue ASTERISK-16456) Reported by: schmidts Patches: sched.c.patch uploaded by schmidts (license 1077) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=281575 By: Digium Subversion (svnbot) 2010-08-10 13:05:39 Repository: asterisk Revision: 281576 _U trunk/ U trunk/main/sched.c ------------------------------------------------------------------------ r281576 | russell | 2010-08-10 13:05:38 -0500 (Tue, 10 Aug 2010) | 23 lines Merged revisions 281575 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................ r281575 | russell | 2010-08-10 13:05:07 -0500 (Tue, 10 Aug 2010) | 16 lines Merged revisions 281574 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........ r281574 | russell | 2010-08-10 13:04:32 -0500 (Tue, 10 Aug 2010) | 9 lines Don't move the time threshold for running scheduled events on every iteration. Instead, only calculate the time threshold each time ast_sched_runq() is called. (closes issue ASTERISK-16456) Reported by: schmidts Patches: sched.c.patch uploaded by schmidts (license 1077) ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=281576 | ||