From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17385 invoked by alias); 4 Mar 2011 19:52:57 -0000 Received: (qmail 17375 invoked by uid 22791); 4 Mar 2011 19:52:56 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Mar 2011 19:52:44 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p24Jqh0L012948 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 4 Mar 2011 14:52:43 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p24JqgvJ025229; Fri, 4 Mar 2011 14:52:42 -0500 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p24Jqgax030578; Fri, 4 Mar 2011 14:52:42 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id C223737967C; Fri, 4 Mar 2011 12:52:41 -0700 (MST) From: Tom Tromey To: gdb-patches@sourceware.org Subject: RFC: lazily call save_current_space_and_thread Date: Fri, 04 Mar 2011 19:52:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2011-03/txt/msg00320.txt.bz2 I would appreciate comments on this. I have been working on making the "make check" multi-inferior scenario work better in gdb. The premise in this situation is that most inferiors are not actually interesting, so we want to read debuginfo as lazily as possible. While debugging causes of debuginfo loading, I found that calling save_current_space_and_thread is actually quite expensive. This is because it calls make_cleanup_restore_current_thread, which looks up the selected frame, which can cause all kinds of work. This patch changes the breakpoint insertion code to lazily call save_current_space_and_thread. Built and regtested on x86-64 (compile farm). It has occurred to me that perhaps this is just a hack, and that a better fix would be to somehow maintain the selected frame per-thread. I'd appreciate comments on this as well. A more general point here is that sometimes there are hints about future directions of gdb -- in the comments, in bugzilla -- but it is hard to evaluate whether those directions still make sense. I think we'd all benefit from more clarity about the design wish-list. I hear complaints about this quite a bit, and I think this lack of clarity makes potential contributors more reluctant to attempt bigger projects; nobody wants to invest a lot of time on an idea that might be rejected. Tom 2011-03-04 Tom Tromey * breakpoint.c (insert_breakpoint_locations): Lazily call save_current_space_and_thread. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 623effa..5cf3fcd 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1842,13 +1842,18 @@ insert_breakpoint_locations (void) struct ui_file *tmp_error_stream = mem_fileopen (); struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_error_stream); - + + /* We lazily save the current program space. Saving the current + program space is reasonably expensive in the multi-inferior case, + and in many situations we don't need to do it. Comparing a + cleanup against NULL is usually forbidden, but in this case we + have already made a cleanup, so we know it is safe. */ + struct cleanup *current_space_cleanup = NULL; + /* Explicitly mark the warning -- this will only be printed if there was an error. */ fprintf_unfiltered (tmp_error_stream, "Warning:\n"); - save_current_space_and_thread (); - ALL_BP_LOCATIONS (bl, blp_tmp) { if (!should_be_inserted (bl) || bl->inserted) @@ -1861,6 +1866,8 @@ insert_breakpoint_locations (void) && !valid_thread_id (bl->owner->thread)) continue; + if (current_space_cleanup == NULL) + current_space_cleanup = save_current_space_and_thread (); switch_to_program_space_and_thread (bl->pspace); /* For targets that support global breakpoints, there's no need