[Bug-wget] [Bug-Wget] Release in the next couple of days?
Hi, Are we planning a new release in the next couple of days? I think the source looks good and make dist / make distcheck seem to be in good shape right now. For this release we have a few bug fixes, a couple of new command line switches and fixes for some memory leaks. There's also the removal of SSLv3 from being used by default. -- Thanking You, Darshit Shah pgpYqdUD9bP_v.pgp Description: PGP signature
Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings
Pär, thanks for your work. *BUT* speed does not really matter here. My intention was to show code that is much more readable than the current implementation which is unnecessary complicated and not even understandable by the LLVM/clang analyzer. That piece of code should be replaced. FYI, if you want to work on CPU cycle optimization, use valgrind -- tool=callgrind for your wget command to be polished. The resulting file can be viewed with e.g. kcachegrind. Tim Am Freitag, 24. Oktober 2014, 19:27:43 schrieb Pär Karlsson: > Well, I wrote a little benchmark and implemented them all in a hastily > thrown together little program. > > Basically, it tests three different strings against all three > implementations of the functions (but it uses malloc instead of xmalloc, > because I didn't want to throw in the whole of gnulib there too): > > The results from 1 million iterations on each string (on an AMD Athlon(tm) > Dual Core Processor 4850e) are as follows: > > feinorgh@elektra ~/code $ make strtest > cc strtest.c -o strtest > feinorgh@elektra ~/code $ ./strtest > Result concat_strings: avg 0.0018 s, total 0.18253400 s > Result concat_strings_new: avg 0.0025 s, total 0.25143400 s > Result concat_strings_tim: avg 0.0036 s, total 0.36166900 s > Result concat_strings: avg 0.0015 s, total 0.15230500 s > Result concat_strings_new: avg 0.0022 s, total 0.22280100 s > Result concat_strings_tim: avg 0.0027 s, total 0.27337900 s > Result concat_strings: avg 0.0073 s, total 0.72672500 s > Result concat_strings_new: avg 0.0066 s, total 0.66113200 s > Result concat_strings_tim: avg 0.0148 s, total 1.47995200 s > feinorgh@elektra ~/code $ gcc -O3 -march=native -o strtest strtest.c -Wall > feinorgh@elektra ~/code $ ./strtest > Result concat_strings: avg 0.0013 s, total 0.12796300 s > Result concat_strings_new: avg 0.0020 s, total 0.20472700 s > Result concat_strings_tim: avg 0.0016 s, total 0.16481100 s > Result concat_strings: avg 0.0011 s, total 0.10742400 s > Result concat_strings_new: avg 0.0018 s, total 0.18417900 s > Result concat_strings_tim: avg 0.0013 s, total 0.12504500 s > Result concat_strings: avg 0.0059 s, total 0.58840200 s > Result concat_strings_new: avg 0.0054 s, total 0.53843800 s > Result concat_strings_tim: avg 0.0068 s, total 0.68416400 s > feinorgh@elektra ~/code $ gcc -g -march=native -o strtest strtest.c -Wall > feinorgh@elektra ~/code $ valgrind ./strtest > ==3802== Memcheck, a memory error detector > ==3802== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. > ==3802== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info > ==3802== Command: ./strtest > ==3802== > Result concat_strings: avg 0.0754 s, total 7.54378500 s > Result concat_strings_new: avg 0.1080 s, total 10.80041900 s > Result concat_strings_tim: avg 0.0900 s, total 8.99982200 s > Result concat_strings: avg 0.0652 s, total 6.52148200 s > Result concat_strings_new: avg 0.0985 s, total 9.85281200 s > Result concat_strings_tim: avg 0.0770 s, total 7.69649600 s > Result concat_strings: avg 0.2109 s, total 21.08910100 s > Result concat_strings_new: avg 0.2570 s, total 25.69685100 s > Result concat_strings_tim: avg 0.2541 s, total 25.40761200 s > ==3802== > ==3802== HEAP SUMMARY: > ==3802== in use at exit: 0 bytes in 0 blocks > ==3802== total heap usage: 13,000,000 allocs, 13,000,000 frees, > 743,000,000 bytes allocated > ==3802== > ==3802== All heap blocks were freed -- no leaks are possible > ==3802== > ==3802== For counts of detected and suppressed errors, rerun with: -v > ==3802== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1) > > All three implementations perform correctly as far as I can tell (they give > identical results), and behave well regarding memory management (as long as > the allocated space is free():d afterwards). > > I tested it with gperf too, and it kind of indicated the same results, the > original implementation is the fastest. > > I'm almost ashamed to attach the benchmark program, since it's so clumsily > (and copy-pastily) put together, but for completeness, I attach it anyway > (strtest.c.gz). > > My conclusion is: the current implementation is the best and most > efficient; it does everything it's supposed to do, and it does it quicker > (at least on my machine) than any of the given alternatives. For 5 > arguments or less, it's the most efficient implementation, and nowhere in > the current codebase is there a place where more than 5 strings are used... > to my knowledge :-) There's really nothing wrong with it, IMO :-) > > Best regards, > > /Pär > > 2014-10-23 22:01 GMT+02:00 Tim Rühsen : > > Am Dienstag, 21. Oktober 2014, 16:49:12 schrieb Yousong Zhou: > > > On 21 October 2014 16:17, Pär Karlsson wrote: > > > > Yes, you are right, of course. Looking through the original > > > > implementation > >
Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings
Well, I wrote a little benchmark and implemented them all in a hastily thrown together little program. Basically, it tests three different strings against all three implementations of the functions (but it uses malloc instead of xmalloc, because I didn't want to throw in the whole of gnulib there too): The results from 1 million iterations on each string (on an AMD Athlon(tm) Dual Core Processor 4850e) are as follows: feinorgh@elektra ~/code $ make strtest cc strtest.c -o strtest feinorgh@elektra ~/code $ ./strtest Result concat_strings: avg 0.0018 s, total 0.18253400 s Result concat_strings_new: avg 0.0025 s, total 0.25143400 s Result concat_strings_tim: avg 0.0036 s, total 0.36166900 s Result concat_strings: avg 0.0015 s, total 0.15230500 s Result concat_strings_new: avg 0.0022 s, total 0.22280100 s Result concat_strings_tim: avg 0.0027 s, total 0.27337900 s Result concat_strings: avg 0.0073 s, total 0.72672500 s Result concat_strings_new: avg 0.0066 s, total 0.66113200 s Result concat_strings_tim: avg 0.0148 s, total 1.47995200 s feinorgh@elektra ~/code $ gcc -O3 -march=native -o strtest strtest.c -Wall feinorgh@elektra ~/code $ ./strtest Result concat_strings: avg 0.0013 s, total 0.12796300 s Result concat_strings_new: avg 0.0020 s, total 0.20472700 s Result concat_strings_tim: avg 0.0016 s, total 0.16481100 s Result concat_strings: avg 0.0011 s, total 0.10742400 s Result concat_strings_new: avg 0.0018 s, total 0.18417900 s Result concat_strings_tim: avg 0.0013 s, total 0.12504500 s Result concat_strings: avg 0.0059 s, total 0.58840200 s Result concat_strings_new: avg 0.0054 s, total 0.53843800 s Result concat_strings_tim: avg 0.0068 s, total 0.68416400 s feinorgh@elektra ~/code $ gcc -g -march=native -o strtest strtest.c -Wall feinorgh@elektra ~/code $ valgrind ./strtest ==3802== Memcheck, a memory error detector ==3802== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==3802== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info ==3802== Command: ./strtest ==3802== Result concat_strings: avg 0.0754 s, total 7.54378500 s Result concat_strings_new: avg 0.1080 s, total 10.80041900 s Result concat_strings_tim: avg 0.0900 s, total 8.99982200 s Result concat_strings: avg 0.0652 s, total 6.52148200 s Result concat_strings_new: avg 0.0985 s, total 9.85281200 s Result concat_strings_tim: avg 0.0770 s, total 7.69649600 s Result concat_strings: avg 0.2109 s, total 21.08910100 s Result concat_strings_new: avg 0.2570 s, total 25.69685100 s Result concat_strings_tim: avg 0.2541 s, total 25.40761200 s ==3802== ==3802== HEAP SUMMARY: ==3802== in use at exit: 0 bytes in 0 blocks ==3802== total heap usage: 13,000,000 allocs, 13,000,000 frees, 743,000,000 bytes allocated ==3802== ==3802== All heap blocks were freed -- no leaks are possible ==3802== ==3802== For counts of detected and suppressed errors, rerun with: -v ==3802== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1) All three implementations perform correctly as far as I can tell (they give identical results), and behave well regarding memory management (as long as the allocated space is free():d afterwards). I tested it with gperf too, and it kind of indicated the same results, the original implementation is the fastest. I'm almost ashamed to attach the benchmark program, since it's so clumsily (and copy-pastily) put together, but for completeness, I attach it anyway (strtest.c.gz). My conclusion is: the current implementation is the best and most efficient; it does everything it's supposed to do, and it does it quicker (at least on my machine) than any of the given alternatives. For 5 arguments or less, it's the most efficient implementation, and nowhere in the current codebase is there a place where more than 5 strings are used... to my knowledge :-) There's really nothing wrong with it, IMO :-) Best regards, /Pär 2014-10-23 22:01 GMT+02:00 Tim Rühsen : > Am Dienstag, 21. Oktober 2014, 16:49:12 schrieb Yousong Zhou: > > On 21 October 2014 16:17, Pär Karlsson wrote: > > > Yes, you are right, of course. Looking through the original > implementation > > > again, it seems water tight. clang probably complains about the > > > uninitialized values above argcount in saved_lengths[], that are never > > > reached. > > > > > > The precalculated strlen:s saved is likely only an optimization(?) > > > attempt, > > > I suppose. > > > > Yes. Grepping through the code shows that currently there is no > > invocation of concat_strings() having more than 5 arguments. > > > > > Still, it seems wasteful to set up two complete loops with va_arg, and > > > considering what this function actually does, I wonder if not > s(n)printf > > > should be used instead of this function? :-) > > > > I think concat_strings() is more tight and readable than multiple > > strlen() + malloc
