From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8632 invoked by alias); 3 Dec 2017 17:50:18 -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 8615 invoked by uid 89); 3 Dec 2017 17:50:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=scripting, pronounced, variation, representative X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 03 Dec 2017 17:50:16 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id vB3HoAvE001438 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 3 Dec 2017 12:50:15 -0500 Received: by simark.ca (Postfix, from userid 112) id 6B8411E585; Sun, 3 Dec 2017 12:50:10 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id CF91A1E02D; Sun, 3 Dec 2017 12:49:49 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 03 Dec 2017 17:50:00 -0000 From: Simon Marchi To: Pedro Alves Cc: gdb-patches@sourceware.org, Tom Tromey , Simon Marchi Subject: Re: [PATCH] C++-ify parse_format_string In-Reply-To: <315d390f-5b5d-f077-e730-ae4535729557@redhat.com> References: <87h8tivyo4.fsf@tromey.com> <20171202203133.10770-1-simon.marchi@polymtl.ca> <315d390f-5b5d-f077-e730-ae4535729557@redhat.com> Message-ID: <8990a259a6bc815854c4645a9fcef3ad@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.2 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 3 Dec 2017 17:50:10 +0000 X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00060.txt.bz2 On 2017-12-03 09:12, Pedro Alves wrote: > Use plain "time" first. (You may have shifted work > out of parse_format_string.) > > With your test script as is, I get around: > > real 0m1.053s > user 0m0.985s > sys 0m0.068s > > I like a testcase that runs a bit longer in order to have a > better signal/noise ratio. IMO 1 second is too short. Bumping the > number of iterations 10x (to 200000), I get (representative > of a few runs), > > before patch: > > real 0m9.432s > user 0m8.978s > sys 0m0.444s > > after patch: > > real 0m10.322s > user 0m9.854s > sys 0m0.454s > > there's some variation across runs, but after-patch compared > to before-patch is consistently around 7%-11% slower, for me. Indeed, I see similar results (except 200000 iterations take 30 seconds for me with my 10 years old CPU :)). > Looking at "perf report -g", we see we're very inefficient > while handling this specific use case, spending a lot of time > in the C parser and in fprintf_filtered. If those paths > were tuned a bit the relative slowdown of parse_format_string > would probably be more pronounced. > > Note that parse_format_string is used to implement target-side > dprintf, in gdbserver/ax.c:ax_printf. This means that a slow > down here affects inferior execution time too, which may be > a less-contrived scenario. I agree that we should not make parse_format_string slow just for fun, but in this particular case I think we should avoid parsing the format string on the fast path. It should be possible to chew it to pieces earlier (when installing the breakpoint) and just access the pieces when we hit the dprintf. > My point is still the same as ever -- it's not possible > upfront to envision all the use cases users throw at us, > given gdb scripting, etc. So if easy, I think it's better > to avoid premature pessimization: > > https://sourceware.org/ml/gdb-patches/2017-06/msg00506.html The article you link defines premature pessimization as "when equivalently complex code would be faster". In this case, the code is more complex (though maybe not by much) with a shared buffer, and I was wondering if that complexity was really worth the speedup, especially for something not expected to be on a fast path. But since it's already implemented like this and we know it's faster, I agree it feels like a step backwards to remove it. So I have no problem going with Tom's patch. Simon