John Van Essen
2003-Apr-08 18:49 UTC
[Patch] Require extra --stats to emit heap statistics
Since the heap statistics were added, I have viewed thousands of rsync reports (with --verbose and --stats), and not once have I had a need for them. So for me, they're just noise. Here is a 3-part patch to v2.5.6 that treats --stats in a similar fashion to --verbose in that additional --stats will get you more statistics. For this initial case, there's only one additional level of statistics (for the heap) but it could be expanded in the future. I have also included formatting changes so that the 3 stats sections consistently emit a single newline prior to emitting the statistics, thus visually separating them from whatever was displayed previously. --- options.c.orig Mon Jan 27 19:11:57 2003 +++ options.c Sun Mar 30 15:11:58 2003 @@ -350,7 +350,7 @@ {"compress", 'z', POPT_ARG_NONE, &do_compression , 0, 0, 0 }, {"daemon", 0, POPT_ARG_NONE, &am_daemon , 0, 0, 0 }, {"no-detach", 0, POPT_ARG_NONE, &no_detach , 0, 0, 0 }, - {"stats", 0, POPT_ARG_NONE, &do_stats , 0, 0, 0 }, + {"stats", 0, POPT_ARG_NONE, 0, OPT_STATS, 0, 0 }, {"progress", 0, POPT_ARG_NONE, &do_progress , 0, 0, 0 }, {"partial", 0, POPT_ARG_NONE, &keep_partial , 0, 0, 0 }, {"ignore-errors", 0, POPT_ARG_NONE, &ignore_errors , 0, 0, 0 }, @@ -555,6 +555,10 @@ if (frommain) quiet++; break; + case OPT_STATS: + do_stats++; + break; + case 'a': recurse=1; #if SUPPORT_LINKS --- main.c.orig Mon Jan 27 21:05:53 2003 +++ main.c Fri Apr 4 00:06:41 2003 @@ -82,7 +82,7 @@ extern int remote_version; int send_stats; - if (do_stats) { + if (do_stats > 1) { /* These come out from every process */ show_malloc_stats(); show_flist_stats(); @@ -139,12 +139,12 @@ rprintf(FINFO,"File list size: %d\n", stats.flist_size); rprintf(FINFO,"Total bytes written: %.0f\n", (double)stats.total_written); - rprintf(FINFO,"Total bytes read: %.0f\n\n", + rprintf(FINFO,"Total bytes read: %.0f\n", (double)stats.total_read); } if (verbose || do_stats) { - rprintf(FINFO,"wrote %.0f bytes read %.0f bytes %.2f bytes/sec\n", + rprintf(FINFO,"\nwrote %.0f bytes read %.0f bytes %.2f bytes/sec\n", (double)stats.total_written, (double)stats.total_read, (stats.total_written+stats.total_read)/(0.5 + (t-starttime))); @@ -171,7 +171,7 @@ mi = mallinfo(); - rprintf(FINFO, RSYNC_NAME "[%d] (%s%s%s) heap statistics:\n", + rprintf(FINFO, "\n" RSYNC_NAME "[%d] (%s%s%s) heap statistics:\n", getpid(), am_server ? "server " : "", am_daemon ? "daemon " : "", --- rsync.1.orig Mon Jan 27 19:11:57 2003 +++ rsync.1 Sun Mar 30 15:00:44 2003 @@ -889,7 +889,8 @@ .IP "\fB--stats\fP" This tells rsync to print a verbose set of statistics on the file transfer, allowing you to tell how effective the rsync -algorithm is for your data\&. +algorithm is for your data\&. An additional --stats will also +print statistics on heap memory allocation (if available)\&. .IP .IP "\fB--partial\fP" By default, rsync will delete any partially -- John Van Essen Univ of MN Alumnus <vanes002@umn.edu>
On Tue, Apr 08, 2003 at 03:49:00AM -0500, John Van Essen wrote:> Since the heap statistics were added, I have viewed thousands of rsync > reports (with --verbose and --stats), and not once have I had a need > for them. So for me, they're just noise.I agree with you on that. Then again i've never run out of memory with rsync.> Here is a 3-part patch to v2.5.6 that treats --stats in a similar > fashion to --verbose in that additional --stats will get you more > statistics. For this initial case, there's only one additional level > of statistics (for the heap) but it could be expanded in the future.Please no! The -vvvvvvvvvvv is bad enough, don't turn stats into the same abominat^Wawkwardness. I have mentioned before that i would like to see selectable logging. Stats would fit into such a framework. At the moment we have three statistical summaries. It should be possible to select them individually.> I have also included formatting changes so that the 3 stats sections > consistently emit a single newline prior to emitting the statistics, > thus visually separating them from whatever was displayed previously.That is a good adjustment.
John Van Essen
2003-Apr-10 20:38 UTC
[Patch] Require extra --stats to emit heap statistics
On Tue, 8 Apr 2003, jw schultz <jw@pegasys.ws> wrote:>On Tue, Apr 08, 2003 at 03:49:00AM -0500, John Van Essen wrote: >> Since the heap statistics were added, I have viewed thousands of rsync >> reports (with --verbose and --stats), and not once have I had a need >> for them. So for me, they're just noise. > >I agree with you on that. Then again i've never run out of >memory with rsync.The heap stats are more of a debugging aid than generally useful info, so they would be more appropriate for a higher level of verbosity.>> Here is a 3-part patch to v2.5.6 that treats --stats in a similar >> fashion to --verbose in that additional --stats will get you more >> statistics. For this initial case, there's only one additional level >> of statistics (for the heap) but it could be expanded in the future. > >Please no! The -vvvvvvvvvvv is bad enough, don't turn stats >into the same abominat^Wawkwardness. > >I have mentioned before that i would like to see selectable >logging. Stats would fit into such a framework. At the moment >we have three statistical summaries. It should be possible >to select them individually.If you don't want to use^H^H^Habuse the --stats argument, then how about just requiring more than one -v to emit the heap statistics? Changing if (verbose) to if (verbose > 1) would do it. Are there any compelling reasons to emit them at the single -v level? -- John Van Essen <vanes002@umn.edu>