As mentioned in Part 01 of the project a very noticeable and easy to apply optimization was the -O3 compiler flag as before there was no optimization done at all. This seemed like the -O3 flag was simply forgotten or overlooked, however as I attempted to apply the flag I came across some issues that were disguised by only applying minimal optimization. When I first added the flag to both the STATIC and #else macros in the Makefile I got this issue in the strdup.h file in deps/strdup:
fatal error: expected identifier or '(' before '__extension__'
After some research and code manipulation I have discovered there are many work arounds to this problem. For one, I could use a different Makefile for the Sha1 function such as in the development version, however this seems impractical and is more like a temporary band-aid solution. Secondly I could alter the strdup function is strdup.h so it does not share the same name as the same function present in string.h which I pieced together from here and looking at the function dependancies. Lastly, I thought I could change the macros to check for STRDUP_H and if it does not find it use the default strdup function already defined in string.h
In order to make a good solution that could potentially be implemented I opted to try option two and three. Option two was safer, however meant that every single call to strdup had to be changed. This in itself is not bad, however I began to think about the dependancies the clib package manager might install and these may also call strdup. At this point I switched directions and looked at the implementation of the strdup they made and it did not appear to do anythig different than the regular implementation at cppreference. I thought redefining the macros would be rather simple and it was, I renamed the strdup macros to look for the strdup.h file and include the string.h file if it could not find it. I made an else to use their implementation if there is no standard c library defined on the system. This too, does not make sense because there are other standard C .h files included that require the GNU C library to exist so there is no reason that strdup.h should not exist anyway.
A side by side comparison of the orginal strdup.h and strdup.c code and my code are provided here:
strdup.h
Mine: here
Standard: here
strdup.c
Mine: here
Standard: here
Benchmarking:
To benchmark the new speed I used clib-install to get the sha1 function, used the make command to ensure all the default flags plus the new -O3 optimization were applied, and then yanked out two versions of the function for testing. Both used the default Makefile flags for clib with the exception of one using the new -O3 flag. For comparison purposes I ran each test 5 times against all 6 test vectors provided in the test.c file. I made the following graph below, that demonstrates the speed difference, particularly in vector 6 which is the most intensive due to it's repetition:
It is clear that the optimization was a success and reduced the execution time by over 50%.
Testing:
In order to ensure that with the way I altered the build options I did not break anything in the process I ran some tests. Firstly I scrutinized the make command for any warnings and/or errors. These were mentioned previously and resolved with macro changes. Secondly, I used the make test option already provided in the Makefile. When I first ran make test with the vanilla version of clib I received some warnings regarding the same strdup function such as implicit delarations and casting errors. When I ran make test after changing the flags I received the same warnings which suggested no interference with code already established. Lastly, to be absolutely sure I used clib-install to donwload many other packages followed by make clean and make. This yielded no errors or warnings of any kind.
In conclusion, I would recommend this vital change get pushed upstream. It dramatically improves speed with little changes needing to be made, those of which should be made anyway to fix the strdup function warnings to errors with speed up. It has been safely tested, and yields no new warnings or errors. This change also brings uniformity to the strdup function or at least brings the opportunity to make it better established. I think this change is not only a speed increase but an opportunity to better the clib project as a whole.
fatal error: expected identifier or '(' before '__extension__'
After some research and code manipulation I have discovered there are many work arounds to this problem. For one, I could use a different Makefile for the Sha1 function such as in the development version, however this seems impractical and is more like a temporary band-aid solution. Secondly I could alter the strdup function is strdup.h so it does not share the same name as the same function present in string.h which I pieced together from here and looking at the function dependancies. Lastly, I thought I could change the macros to check for STRDUP_H and if it does not find it use the default strdup function already defined in string.h
In order to make a good solution that could potentially be implemented I opted to try option two and three. Option two was safer, however meant that every single call to strdup had to be changed. This in itself is not bad, however I began to think about the dependancies the clib package manager might install and these may also call strdup. At this point I switched directions and looked at the implementation of the strdup they made and it did not appear to do anythig different than the regular implementation at cppreference. I thought redefining the macros would be rather simple and it was, I renamed the strdup macros to look for the strdup.h file and include the string.h file if it could not find it. I made an else to use their implementation if there is no standard c library defined on the system. This too, does not make sense because there are other standard C .h files included that require the GNU C library to exist so there is no reason that strdup.h should not exist anyway.
A side by side comparison of the orginal strdup.h and strdup.c code and my code are provided here:
strdup.h
Mine: here
Standard: here
strdup.c
Mine: here
Standard: here
Benchmarking:
To benchmark the new speed I used clib-install to get the sha1 function, used the make command to ensure all the default flags plus the new -O3 optimization were applied, and then yanked out two versions of the function for testing. Both used the default Makefile flags for clib with the exception of one using the new -O3 flag. For comparison purposes I ran each test 5 times against all 6 test vectors provided in the test.c file. I made the following graph below, that demonstrates the speed difference, particularly in vector 6 which is the most intensive due to it's repetition:
It is clear that the optimization was a success and reduced the execution time by over 50%.
Testing:
In order to ensure that with the way I altered the build options I did not break anything in the process I ran some tests. Firstly I scrutinized the make command for any warnings and/or errors. These were mentioned previously and resolved with macro changes. Secondly, I used the make test option already provided in the Makefile. When I first ran make test with the vanilla version of clib I received some warnings regarding the same strdup function such as implicit delarations and casting errors. When I ran make test after changing the flags I received the same warnings which suggested no interference with code already established. Lastly, to be absolutely sure I used clib-install to donwload many other packages followed by make clean and make. This yielded no errors or warnings of any kind.
In conclusion, I would recommend this vital change get pushed upstream. It dramatically improves speed with little changes needing to be made, those of which should be made anyway to fix the strdup function warnings to errors with speed up. It has been safely tested, and yields no new warnings or errors. This change also brings uniformity to the strdup function or at least brings the opportunity to make it better established. I think this change is not only a speed increase but an opportunity to better the clib project as a whole.
Comments
Post a Comment