change log for rtems (2010-06-22)

[Joel Sherrill]

On 06/23/2010 09:57 AM, Joel Sherrill wrote:
> On 06/23/2010 07:23 AM, Sebastian Huber wrote:
>    
>> On 06/23/2010 02:16 PM, Joel Sherrill wrote:
>>
>>      
>>> On 06/22/2010 08:49 PM, Chris Johns wrote:
>>>
>>>        
>>>> On 23/06/10 6:12 AM, rtems-vc at rtems.org wrote:
>>>>
>>>>
>>>>          
>>>>> diff -u rtems/cpukit/libcsupport/include/rtems/libio_.h:1.37
>>>>> rtems/cpukit/libcsupport/include/rtems/libio_.h:1.38
>>>>> --- rtems/cpukit/libcsupport/include/rtems/libio_.h:1.37    Mon Jun
>>>>> 14 08:35:45 2010
>>>>> +++ rtems/cpukit/libcsupport/include/rtems/libio_.h    Tue Jun 22
>>>>> 15:03:41 2010
>>>>> @@ -148,13 +148,7 @@
>>>>>       *  Macro to free a node.
>>>>>       */
>>>>>
>>>>> -#define rtems_filesystem_freenode( _node ) \
>>>>> -  do { \
>>>>> -    if ( (_node)->ops )\
>>>>> -      if ( (_node)->ops->freenod_h ) \
>>>>> -        (*(_node)->ops->freenod_h)( (_node) ); \
>>>>> -  } while (0)
>>>>> -
>>>>> +void rtems_filesystem_freenode( rtems_filesystem_location_info_t*
>>>>> node );
>>>>>
>>>>>
>>>>>
>>>>>            
>>>> Why has this been done ?
>>>>
>>>> This code is a hot spot in the file system. Not all file systems need to
>>>> free a node but they now suffer an extra call. Maybe the overhead is not
>>>> too much but it does add up.
>>>>
>>>>
>>>>
>>>>          
>>> It lead to horrible branch patch explosion and code size.   It introduced
>>> a lot of paths that had to tested.  Look at the top two entries here:
>>>
>>> http://www.rtems.org/ftp/pub/rtems/people/joel/coverage/erc32.html#OsPD
>>>
>>> Code coverage went up about 4%!!! And the number of branches which
>>> are executed but we have only taken one direction (e.g. always taken
>>> or never taken) dropped from 727 to 626.
>>>
>>>        
>> We should consider to forbid NULL pointers for ops tables and handlers.
>> Instead we should provide reasonable default handlers.  This would eliminate a
>> lot of ifs and make the code more readable.
>>
>>
>>      
> I was thinking of the same thing.  There are a LOT of places there
> are checks and having standard stubs would really help the code.
>
> As we try to improve testability, this type of thing is a very good
> technique.  Plus it also improves readability IMO.
>
>    
I hate to reply to myself but this would eliminate a lot of
branches that have to be tested and the need for the corresponding
test code.   FWIW libcsupport has 46 tests for NULL op handlers

$ grep "\!.*op" *.c | wc -l
46

Now look at the summary for the coverage run last night

Bytes Analyzed           : 120464
Bytes Not Executed       : 9860
Percentage Executed      : 91.81
Percentage Not Executed  : 8.185
Uncovered ranges found   : 322
Total branches found     : 2742
Uncovered branches found : 626
    243 branches always taken
    383 branches never taken



Let's assume that only 40 of the ones shown by grep are
eliminated.  That is at least 40 fewer branches. :)

--joel