From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12965 invoked by alias); 2 Sep 2010 18:40:36 -0000 Received: (qmail 12940 invoked by uid 22791); 2 Sep 2010 18:40:34 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 02 Sep 2010 18:40:29 +0000 Received: from kpbe18.cbf.corp.google.com (kpbe18.cbf.corp.google.com [172.25.105.82]) by smtp-out.google.com with ESMTP id o82IeQHi018179 for ; Thu, 2 Sep 2010 11:40:26 -0700 Received: from vws8 (vws8.prod.google.com [10.241.21.136]) by kpbe18.cbf.corp.google.com with ESMTP id o82IdsJ5007175 for ; Thu, 2 Sep 2010 11:40:08 -0700 Received: by vws8 with SMTP id 8so1088383vws.2 for ; Thu, 02 Sep 2010 11:40:01 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.157.136 with SMTP id b8mr5500885vcx.214.1283452799050; Thu, 02 Sep 2010 11:39:59 -0700 (PDT) Received: by 10.220.200.73 with HTTP; Thu, 2 Sep 2010 11:39:58 -0700 (PDT) In-Reply-To: <20100902160216.GA10848@host1.dyn.jankratochvil.net> References: <20100823185008.GA2926@host1.dyn.jankratochvil.net> <20100902160216.GA10848@host1.dyn.jankratochvil.net> Date: Thu, 02 Sep 2010 19:33:00 -0000 Message-ID: Subject: Re: [patch] Fix DW_OP_call2 and DW_OP_call4 for max-cache-age 0 From: Doug Evans To: Jan Kratochvil Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes 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 X-SW-Source: 2010-09/txt/msg00105.txt.bz2 On Thu, Sep 2, 2010 at 9:02 AM, Jan Kratochvil wrote: > On Mon, 23 Aug 2010 21:30:06 +0200, Doug Evans wrote: >> On Mon, Aug 23, 2010 at 11:50 AM, Jan Kratochvil wrote: >> > as discussed on #gdb when you set max-cache-age 0 DW_OP_call{2,4} cras= hed GDB. > [...] >> > --- a/gdb/dwarf2read.c >> > +++ b/gdb/dwarf2read.c >> > @@ -1636,6 +1636,11 @@ dw2_do_instantiate_symtab (struct objfile *objf= ile, >> > =A0{ >> > =A0 struct cleanup *back_to; >> > >> > + =A0/* Age the cache, releasing compilation units that have not been = used >> > + =A0 =A0 recently. =A0Age them first so that we do not age out the re= quested PER_CU >> > + =A0 =A0 unit if DWARF2_MAX_CACHE_AGE is too low. =A0*/ >> > + =A0age_cached_comp_units (); >> >> Aging cached units first feels weird (if not wrong at least weird); we >> may toss out something we're about to want. >> At the least IWBN to elaborate on why this fixes things. > > As otherwise we will age out what we have found (on max-cache-age 0). Ah. Still, dw2_do_instantiate_symtab seems like the wrong tool for the job here. Its job is to instantiate a symtab, it currently doesn't guarantee it will leave the CU read in when finished, and adding that guarantee doesn't feel right. Assuming (and I don't know dwarf2_fetch_die_location_block well) just needs the dies and not a symtab, how about moving this bit of code to its own function, and calling it from both dw2_do_instantiate_symtab and dwarf2_fetch_die_location_block. if (per_cu->from_debug_types) read_signatured_type_at_offset (objfile, per_cu->offset); else load_full_comp_unit (per_cu, objfile); I haven't thought it through (e.g. it may need a bit of glue), but it feels like a better approach. > One could forbid value zero for max-cache-age but that also does not seem > right to me. max-cache-age =3D=3D 0 is defined to disable the cache. It's a useful test vehicle, and I don't see any reason to disallow it either. > There is such a general cleanup moment when GDB is fully idle > - prepare_execute_command() - shouldn't age_cached_comp_units be called t= here? I don't know. Or as a cleanup (either via a cleanup itself, or as part of some top level thing akin to whatever you'd do in prepare_execute_command. making use of an existing facility (make_cleanup) would be preferable of course, assuming it's the way to go)? It feels better to do this at the end of a command, not before. > But that way sooner or later we will age out every CU. =A0This may occur = a bit > even nowadays, the default value 5 is also very low. =A0max-cache-age as = "how > long" is IMO not userful to the user. =A0There could be more a setting "h= ow > many" CUs can be loaded at once. =A0CU age would be then just an internal > indicator to maintain the count under the "how many" limit. A better measure may be memory used (e.g. lots of CUs are ok if they're all relatively small). IWBN to find/collect stats on the distribution of #CUs and sizes. [e.g. can we make some reasonable assumptions so that we don't have to track die memory usage?] > I would change "max-cache-age" to "max-cache-size" and call it from > prepare_execute_command() instead. =A0I will provide a patch if not repli= ed.