| Summary: | ASTERISK-13816: Address device state performance issues in 1.6.1 | ||
| Reporter: | Russell Bryant (russell) | Labels: | |
| Date Opened: | 2009-03-24 13:39:11 | Date Closed: | 2009-03-25 17:07:08 |
| Priority: | Major | Regression? | No |
| Status: | Closed/Complete | Components: | Core/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ||
| Description: | http://reviewboard.digium.com/r/205/ | ||
| Comments: | By: Digium Subversion (svnbot) 2009-03-25 16:57:21 Repository: asterisk Revision: 184339 U trunk/apps/app_minivm.c U trunk/apps/app_voicemail.c U trunk/channels/chan_dahdi.c U trunk/channels/chan_iax2.c U trunk/channels/chan_mgcp.c U trunk/channels/chan_sip.c U trunk/channels/chan_unistim.c U trunk/include/asterisk/_private.h U trunk/include/asterisk/devicestate.h U trunk/include/asterisk/event.h U trunk/include/asterisk/strings.h U trunk/main/asterisk.c U trunk/main/devicestate.c U trunk/main/event.c U trunk/res/ais/evt.c ------------------------------------------------------------------------ r184339 | russell | 2009-03-25 16:57:20 -0500 (Wed, 25 Mar 2009) | 35 lines Improve performance of the ast_event cache functionality. This code comes from svn/asterisk/team/russell/event_performance/. Here is a summary of the changes that have been made, in order of both invasiveness and performance impact, from smallest to largest. 1) Asterisk 1.6.1 introduces some additional logic to be able to handle distributed device state. This functionality comes at a cost. One relatively minor change in this patch is that the extra processing required for distributed device state is now completely bypassed if it's not needed. 2) One of the things that I noticed when profiling this code was that a _lot_ of time was spent doing string comparisons. I changed the way strings are represented in an event to include a hash value at the front. So, before doing a string comparison, we do an integer comparison on the hash. 3) Finally, the code that handles the event cache has been re-written. I tried to do this in a such a way that it had minimal impact on the API. I did have to change one API call, though - ast_event_queue_and_cache(). However, the way it works now is nicer, IMO. Each type of event that can be cached (MWI, device state) has its own hash table and rules for hashing and comparing objects. This by far made the biggest impact on performance. For additional details regarding this code and how it was tested, please see the review request. (closes issue ASTERISK-13816) Reported by: russell Review: http://reviewboard.digium.com/r/205/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=184339 By: Digium Subversion (svnbot) 2009-03-25 17:02:22 Repository: asterisk Revision: 184342 _U branches/1.6.1/ U branches/1.6.1/apps/app_voicemail.c U branches/1.6.1/channels/chan_dahdi.c U branches/1.6.1/channels/chan_iax2.c U branches/1.6.1/channels/chan_mgcp.c U branches/1.6.1/channels/chan_sip.c U branches/1.6.1/channels/chan_skinny.c U branches/1.6.1/channels/chan_unistim.c U branches/1.6.1/include/asterisk/_private.h U branches/1.6.1/include/asterisk/devicestate.h U branches/1.6.1/include/asterisk/event.h U branches/1.6.1/include/asterisk/strings.h U branches/1.6.1/main/asterisk.c U branches/1.6.1/main/devicestate.c U branches/1.6.1/main/event.c U branches/1.6.1/res/ais/evt.c ------------------------------------------------------------------------ r184342 | russell | 2009-03-25 17:02:22 -0500 (Wed, 25 Mar 2009) | 43 lines Merged revisions 184339 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r184339 | russell | 2009-03-25 16:57:19 -0500 (Wed, 25 Mar 2009) | 35 lines Improve performance of the ast_event cache functionality. This code comes from svn/asterisk/team/russell/event_performance/. Here is a summary of the changes that have been made, in order of both invasiveness and performance impact, from smallest to largest. 1) Asterisk 1.6.1 introduces some additional logic to be able to handle distributed device state. This functionality comes at a cost. One relatively minor change in this patch is that the extra processing required for distributed device state is now completely bypassed if it's not needed. 2) One of the things that I noticed when profiling this code was that a _lot_ of time was spent doing string comparisons. I changed the way strings are represented in an event to include a hash value at the front. So, before doing a string comparison, we do an integer comparison on the hash. 3) Finally, the code that handles the event cache has been re-written. I tried to do this in a such a way that it had minimal impact on the API. I did have to change one API call, though - ast_event_queue_and_cache(). However, the way it works now is nicer, IMO. Each type of event that can be cached (MWI, device state) has its own hash table and rules for hashing and comparing objects. This by far made the biggest impact on performance. For additional details regarding this code and how it was tested, please see the review request. (closes issue ASTERISK-13816) Reported by: russell Review: http://reviewboard.digium.com/r/205/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=184342 By: Digium Subversion (svnbot) 2009-03-25 17:07:08 Repository: asterisk Revision: 184343 _U branches/1.6.2/ U branches/1.6.2/apps/app_minivm.c U branches/1.6.2/apps/app_voicemail.c U branches/1.6.2/channels/chan_dahdi.c U branches/1.6.2/channels/chan_iax2.c U branches/1.6.2/channels/chan_mgcp.c U branches/1.6.2/channels/chan_sip.c U branches/1.6.2/channels/chan_unistim.c U branches/1.6.2/include/asterisk/_private.h U branches/1.6.2/include/asterisk/devicestate.h U branches/1.6.2/include/asterisk/event.h U branches/1.6.2/include/asterisk/strings.h U branches/1.6.2/main/asterisk.c U branches/1.6.2/main/devicestate.c U branches/1.6.2/main/event.c U branches/1.6.2/res/ais/evt.c ------------------------------------------------------------------------ r184343 | russell | 2009-03-25 17:07:08 -0500 (Wed, 25 Mar 2009) | 43 lines Merged revisions 184339 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r184339 | russell | 2009-03-25 16:57:19 -0500 (Wed, 25 Mar 2009) | 35 lines Improve performance of the ast_event cache functionality. This code comes from svn/asterisk/team/russell/event_performance/. Here is a summary of the changes that have been made, in order of both invasiveness and performance impact, from smallest to largest. 1) Asterisk 1.6.1 introduces some additional logic to be able to handle distributed device state. This functionality comes at a cost. One relatively minor change in this patch is that the extra processing required for distributed device state is now completely bypassed if it's not needed. 2) One of the things that I noticed when profiling this code was that a _lot_ of time was spent doing string comparisons. I changed the way strings are represented in an event to include a hash value at the front. So, before doing a string comparison, we do an integer comparison on the hash. 3) Finally, the code that handles the event cache has been re-written. I tried to do this in a such a way that it had minimal impact on the API. I did have to change one API call, though - ast_event_queue_and_cache(). However, the way it works now is nicer, IMO. Each type of event that can be cached (MWI, device state) has its own hash table and rules for hashing and comparing objects. This by far made the biggest impact on performance. For additional details regarding this code and how it was tested, please see the review request. (closes issue ASTERISK-13816) Reported by: russell Review: http://reviewboard.digium.com/r/205/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=184343 | ||