From patchwork Wed Jun 19 11:34:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 33207 Received: (qmail 41450 invoked by alias); 19 Jun 2019 11:34:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 41442 invoked by uid 89); 19 Jun 2019 11:34:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=avoided X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Jun 2019 11:34:38 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id BDF0E560B9; Wed, 19 Jun 2019 07:34:36 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id HV80HNJcAp9X; Wed, 19 Jun 2019 07:34:36 -0400 (EDT) Received: from murgatroyd (75-166-12-78.hlrn.qwest.net [75.166.12.78]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 47DFB560B8; Wed, 19 Jun 2019 07:34:36 -0400 (EDT) From: Tom Tromey To: Pedro Alves Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH] Instantiate a single source highlighter References: <20190618194053.7515-1-tromey@adacore.com> Date: Wed, 19 Jun 2019 05:34:35 -0600 In-Reply-To: (Pedro Alves's message of "Tue, 18 Jun 2019 21:20:02 +0100") Message-ID: <87muidhkqc.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 >>>>> "Pedro" == Pedro Alves writes: >> +/* The global source highlight object, or null if one was never >> + constructed. This is stored here rather than in the class so that >> + we don't need to include anything or do conditional compilation in >> + source-cache.h. */ >> +#ifdef HAVE_SOURCE_HIGHLIGHT >> +static srchilite::SourceHighlight *highlighter; >> +#endif Pedro> Should it be a unique_ptr so that valgrind doesn't complain about Pedro> it leaking when gdb exits? Philippe says no, so I didn't make this change. >> + highlighter->setStyleFile("esc.style"); Pedro> Preexisting, but missing space before parens. Fixed. Pedro> To keep the variable's definition and initialization close by, Pedro> I'd add a get_highlighter function: I just moved the global to be a static in the block that uses it. This seemed just as good and avoided more #ifdefs. Here's what I'm checking in. Tom commit ca0c2edb7577c10c3772c8eed8e253971847ccdc Author: Tom Tromey Date: Tue Jun 18 12:18:24 2019 -0600 Instantiate a single source highlighter It occurred to me that there's no reason to make a new source highlighter each time gdb needs to highlight some source code. Instead, a single one can be created and then simply reused each time. This patch implements this idea. Tested on x86-64 Fedora 29. gdb/ChangeLog 2019-06-19 Tom Tromey * source-cache.c (highlighter): New global. (source_cache::get_source_lines): Create a highlighter on demand. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 83f47b25701..1aab438d035 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2019-06-19 Tom Tromey + + * source-cache.c (highlighter): New global. + (source_cache::get_source_lines): Create a highlighter on demand. + 2019-06-18 Tom de Vries PR gdb/24515 diff --git a/gdb/source-cache.c b/gdb/source-cache.c index 2d5b549d971..86efe83bf9a 100644 --- a/gdb/source-cache.c +++ b/gdb/source-cache.c @@ -197,6 +197,13 @@ source_cache::get_source_lines (struct symtab *s, int first_line, std::ifstream input (fullname); if (input.is_open ()) { + /* The global source highlight object, or null if one + was never constructed. This is stored here rather + than in the class so that we don't need to include + anything or do conditional compilation in + source-cache.h. */ + static srchilite::SourceHighlight *highlighter; + if (s->line_charpos == 0) { scoped_fd desc (open_source_file_with_line_charpos (s)); @@ -209,11 +216,15 @@ source_cache::get_source_lines (struct symtab *s, int first_line, use-after-free. */ fullname = symtab_to_fullname (s); } - srchilite::SourceHighlight highlighter ("esc.outlang"); - highlighter.setStyleFile("esc.style"); + + if (highlighter == nullptr) + { + highlighter = new srchilite::SourceHighlight ("esc.outlang"); + highlighter->setStyleFile ("esc.style"); + } std::ostringstream output; - highlighter.highlight (input, output, lang_name, fullname); + highlighter->highlight (input, output, lang_name, fullname); source_text result = { fullname, output.str () }; m_source_map.push_back (std::move (result));