From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14045 invoked by alias); 30 Jun 2016 17:45:48 -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 14028 invoked by uid 89); 30 Jun 2016 17:45:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=stay, transfer X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 30 Jun 2016 17:45:46 +0000 Received: from svr-orw-fem-05.mgc.mentorg.com ([147.34.97.43]) by relay1.mentorg.com with esmtp id 1bIg24-0004mj-BA from Don_Breazeal@mentor.com ; Thu, 30 Jun 2016 10:45:44 -0700 Received: from [172.30.4.68] (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server (TLS) id 14.3.224.2; Thu, 30 Jun 2016 10:45:44 -0700 Subject: Re: [PATCH v2] Optimize memory_xfer_partial for remote To: Pedro Alves , "gdb-patches@sourceware.org" , "qiyaoltc@gmail.com" References: <1467058970-62136-1-git-send-email-donb@codesourcery.com> <6e4787d9-f6d5-641e-ffd5-1dd255806b3b@redhat.com> From: Don Breazeal Message-ID: <57755AC2.9090008@codesourcery.com> Date: Thu, 30 Jun 2016 17:45:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <6e4787d9-f6d5-641e-ffd5-1dd255806b3b@redhat.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00559.txt.bz2 On 6/30/2016 10:06 AM, Pedro Alves wrote: > On 06/27/2016 09:22 PM, Don Breazeal wrote: > >>>> +/* The default implementation for the to_get_memory_xfer_limit method. >>>> + The hard-coded limit here was determined to be a reasonable default >>>> + that eliminated exponential slowdown on very large transfers without >>>> + unduly compromising performance on smaller transfers. */ >>> >>> Where's this coming from? Is this new experimentation you did, >>> or are you talking about Anton's patch? >> >> Both. I did some experimentation to verify that things were significantly >> slower without any memory transfer limit, which they were, although I never >> reproduced the extreme scenario Anton had reported. Presumably the >> performance differences were due to hardware and environment differences. >> Regarding the comment, I thought some explanation of the hard-coded number >> was appropriate. Is there a better or more preferable way to do this, e.g. >> refer to the commit hash, or does it just seem superfluous? > > OK, you didn't mention this experimentation, which left me wondering. > Particularly, the mention of "exponential" is what most made me pause, > as it's a qualifier not mentioned elsewhere. I should have used something like "significant" or "extreme" instead of exponential. > > I guess my main problem with the comment is that by reading it in > isolation, one has no clue of how what would cause the slowdown (normally > transferring more at a time is faster!), and thus how to reevaluate > the default in the future. How about extending to something like: > > /* The default implementation for the to_get_memory_xfer_limit method. > The hard-coded limit here was determined to be a reasonable default > that eliminated exponential slowdown on very large transfers without > unduly compromising performance on smaller transfers. > This slowdown is mostly caused by memory writing routines doing > unnecessary work upfront when large requests end up usually > only partially satisfied. See memory_xfer_partial's handling of > breakpoint shadows. */ > > Actually, I was going to approve this with that change, but another > another thought crossed my mind, sorry... > > I assume you did this experimentation with remote targets? But this default > will never be used with those, so that experimentation is meaningless for > native targets? Actually, the whole capping is probably pointless with > native targets, since there's really no marshalling and thus no limit. > That'd suggest making the target method return "-1" or some such > to indicate there's no limit. WDTY? That makes sense to me. If it returns ULONGEST_MAX then the rest of the patch can stay as-is. Something like this? +/* The default implementation for the to_get_memory_xfer_limit method. + The default limit is essentially "no limit". */ + +static ULONGEST +default_get_memory_xfer_limit (struct target_ops *self) +{ + return ULONGEST_MAX; +} + Thanks --Don