From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5537 invoked by alias); 29 Jul 2013 05:45:22 -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 5527 invoked by uid 89); 29 Jul 2013 05:45:21 -0000 X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL,BAYES_50,KHOP_THREADED,RCVD_VIA_APNIC,RDNS_NONE,SPF_SOFTFAIL autolearn=no version=3.3.1 Received: from Unknown (HELO ozlabs.org) (203.10.76.45) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 29 Jul 2013 05:45:20 +0000 Received: from kryten (ppp121-44-48-234.lns20.syd6.internode.on.net [121.44.48.234]) (using SSLv3 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPSA id 774A22C0100; Mon, 29 Jul 2013 15:45:11 +1000 (EST) Date: Mon, 29 Jul 2013 05:45:00 -0000 From: Anton Blanchard To: Pedro Alves Cc: Luis Machado , gdb-patches@sourceware.org Subject: Re: [PATCH] Improve performance of large restore commands Message-ID: <20130729154457.0e6e6bd7@kryten> In-Reply-To: <51F16780.70408@redhat.com> References: <20130725220858.58184193@kryten> <51F16780.70408@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SW-Source: 2013-07/txt/msg00673.txt.bz2 Hi Pedro, Thanks for the review. > > I noticed a large (100MB) restore took hours to complete. The > > problem is target_xfer_partial repeatedly mallocs and memcpys the > > entire 100MB buffer only to find a small portion of it is actually > > written. > > I think you meant memory_xfer_partial, in the breakpoint shadow > handling, right? I'd prefer pushing the capping close to the > offending malloc/memcpy. I wrote the summary after looking at the perf profile data. It is indeed in memory_xfer_partial and to confirm I reran with a gdb built with -fno-inline: memcpy | --- memcpy target_xfer_partial target_write_with_progress target_write_memory restore_command I've moved the check closer and also added a better comment as Luis suggested. Updated patch below. > We could conceivably change that shadowing algorithm to not malloc > at all. E.g., say, with a memory block like > > |------B------| > start end > > with B being the address where a breakpoint is supposed to be planted, > write block [start,B), then a write for the breakpoint instruction > at B, then another block write for (B,e). > > Or, we could throttle the requested window width up/down depending > on the buffer size returned at each partial transfer. > > I'm not actually suggesting doing this, only explaining why I'd > rather put the cap close to the problem it solves. > target_write_partial is used for other targets objects too, not just > memory. Yes, that definitely makes sense. > > We already cap reads to 4K > > Where exactly? In the target backend, perhaps? I'm not finding a > cap at the target.c level. I've removed this comment. I was looking at target_fileio_read_alloc_1 but it actually starts at 4K and doubles in size as it grows. Anton -- [PATCH] Improve performance of large restore commands I noticed a large (100MB) restore took hours to complete. The problem is memory_xfer_partial repeatedly mallocs and memcpys the entire 100MB buffer for breakpoint shadow handling only to find a small portion of it is actually written. The testcase that originally took hours now takes 50 seconds. -- 2013-07-29 Anton Blanchard * target.c (memory_xfer_partial): Cap write to 4K Index: b/gdb/target.c =================================================================== --- a/gdb/target.c +++ b/gdb/target.c @@ -1669,6 +1669,13 @@ memory_xfer_partial (struct target_ops * void *buf; struct cleanup *old_chain; + /* A large write request is likely to be partially satisfied + by memory_xfer_partial_1. We will continually malloc + and free a copy of the entire write request for breakpoint + shadow handling even though we only end up writing a small + subset of it. Cap writes to 4K to mitigate this. */ + len = min(4096, len); + buf = xmalloc (len); old_chain = make_cleanup (xfree, buf); memcpy (buf, writebuf, len);