From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17418 invoked by alias); 10 Dec 2013 21:12:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 17407 invoked by uid 89); 10 Dec 2013 21:12:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vb0-f49.google.com Received: from Unknown (HELO mail-vb0-f49.google.com) (209.85.212.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 10 Dec 2013 21:12:50 +0000 Received: by mail-vb0-f49.google.com with SMTP id x11so972276vbb.22 for ; Tue, 10 Dec 2013 13:12:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=XdQxqVFbYJd+ogIgg2XnL+NH/BKqkQQpxW1/p9G4Sj4=; b=mjKM2dWeqoRhsvW5xW57ojlznC0G6zbEOP7aWhWaRa8qSvXdmnCLtjCwJPIXlfnMzt LbuGEHm9WbnsGL6wEXg0Kgn1GtajtYYkbhK1KP/xAKre3YSjyAp9X5ipkxEnaCz3y6ri 8UkUTLgi6SgZFKBdIdj7ccqJp4uYKArk7+JKx4yN3t9gxcu2QjiHAd5OHXOkNDC6rbPh VcPBPoX/zUMGo1FKCsHLwlIbNwp+MQwmlpDmNt61Gp3Tf/N1FIOjFaI5iZvRd74yqkJw cCrxFamWTVOWJ0ywqmWeF51PqoQ3//352mYF9rJ2RYFHnmW0FYGmMJz7etXqcR3l3dRS mXmw== X-Gm-Message-State: ALoCoQkcmcS2kjsKd2wm16Ka0STbWmdHbLJqvSMiF3IzFq1AKNltG5xnfMxWKLOfbrcf8gdoQV21sRrkCCR5tQcQ5roZSvGerRSbv+59LpggzaS/uKoIzEO7OBQHFoxSTqp0bqxD7x4Y+X6SP6z/j3bqbdUITdvxM2Cj/06ps4m5TKKPTLHIJ6+DCNIGDhU1kyDFaAscwpB782h/YwXfDjli/v0+W/PjnA== MIME-Version: 1.0 X-Received: by 10.58.232.228 with SMTP id tr4mr872801vec.34.1386709961645; Tue, 10 Dec 2013 13:12:41 -0800 (PST) Received: by 10.52.163.52 with HTTP; Tue, 10 Dec 2013 13:12:41 -0800 (PST) In-Reply-To: <5296DDC9.2090103@codesourcery.com> References: <5296DDC9.2090103@codesourcery.com> Date: Tue, 10 Dec 2013 21:12:00 -0000 Message-ID: Subject: Re: [PATCH] mt set per-command remote-packets on|off From: Doug Evans To: Yao Qi Cc: gdb-patches Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2013-12/txt/msg00390.txt.bz2 blech, gmail again. On Wed, Nov 27, 2013 at 10:08 PM, Yao Qi wrote: > > On 11/28/2013 06:43 AM, Doug Evans wrote: >> >> + >> + if (start_stats->remote_packets_enabled) >> + { >> + long packets_sent, packets_rcvd, bytes_sent, bytes_rcvd; >> + >> + get_remote_packet_stats (&packets_sent, &packets_rcvd, >> + &bytes_sent, &bytes_rcvd); >> + if (packets_sent > 0) >> + { >> + printf_unfiltered (_("#pkts sent: %ld (+%ld)," >> + " #pkts rcvd: %ld (+%ld)\n" >> + "#bytes sent: %ld (+%ld)," >> + " #bytes rcvd: %ld (+%ld)\n"), >> + packets_sent, >> + packets_sent - start_stats->start_packets_sent, >> + packets_rcvd, >> + packets_rcvd - start_stats->start_packets_rcvd, >> + bytes_sent, >> + bytes_sent - start_stats->start_packet_bytes_sent, >> + bytes_rcvd, >> + (bytes_rcvd >> + - start_stats->start_packet_bytes_rcvd)); >> + } >> + } > > > It might not be good to bypass target interface to get the data about > remote target in target independent code. We want to print some statistics of target after each command. For "remote" target, we print statistics on packet, and for other targets, we may want to print something else. Good point. OTOH going that route brings more complication that isn't needed at the moment (rsp perf data *is* needed now). [I finally have some(!) time to return to this.] > > IWBN to add a new interface target_print_statistics, and leave each target to decide what data to print. >From a high level perspective the target collects the statistics, but consumers will want to print them their own way. I didn't want to go "whole hog" yet because it's not clear yet what the different consumers are. [And remote protocol performance is in real need of improvement so I wanted to start solving that problem now.] > > > >> diff --git a/gdb/remote.c b/gdb/remote.c >> index 186c058..429a49c 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -426,6 +426,14 @@ struct remote_state >> >> /* The state of remote notification. */ >> struct remote_notif_state *notif_state; >> + >> + /* Statistics for measuring protocol efficiency. >> + Note: These record data at the protocol level, so to speak. >> + If we're communicating over a flaky channel and have to resent packets >> + that is not accounted for here. It's recommended to track resends, etc. >> + separately. */ >> + long packets_sent, packets_received; >> + long bytes_sent, bytes_received; > > > Use "unsigned long"? How about int? > > IWBN to do statistics in a fine granularity, to count for each frequently-used type of packet. For example, we can count the sent 'm', 'x', and 'g' packet, then we'll get more clues from the the statistics messages. Sure, but that's a finer grained (and vastly more verbose) metric than I'm looking for in the output of "mt set per on". > > Probably we can create a new structure, say "remote_packet_statistics", for these fields.... > > >> }; >> >> /* Private data that we'll store in (struct thread_info)->private. */ >> @@ -629,6 +637,20 @@ get_remote_state (void) >> return get_remote_state_raw (); >> } >> >> +/* Fetch the current packet statistics. */ >> + >> +void >> +get_remote_packet_stats (long *packets_sent, long *packets_received, >> + long *bytes_sent, long *bytes_received) >> +{ > > > ... In this way, the number of arguments of function can be reduced. It was done this way to separate the collector of the data from the printer of the data. If we later collect finer grained stats, callers that want a gross number can still call this, and this function could sum up the pieces. > > >> + struct remote_state *rs = get_remote_state (); >> + >> + *packets_sent = rs->packets_sent; >> + *packets_received = rs->packets_received; >> + *bytes_sent = rs->bytes_sent; >> + *bytes_received = rs->bytes_received; >> +} >> + > > > I am trying to add a python binding to get the statistics of rsp packets, which can be used as a new measurement in perf testsuite. > If one change causes the number of packets increased dramatically, the change may cause some performance regressions in remote debugging. > Looks I can base my work on top of this patch. Indeed, adding rsp stats for the perf testsuite is something I want to see happen. Another consumer of the data.