* [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure
@ 2023-10-12 14:47 Tom de Vries
2023-10-12 14:47 ` [PATCH 2/4] [gdb/cli] Factor out try_source_highlight Tom de Vries
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Tom de Vries @ 2023-10-12 14:47 UTC (permalink / raw)
To: gdb-patches
In function source_cache::ensure we have:
...
std::ostringstream output;
...
contents = output.str();
...
The last line causes an unnecessary string copy.
C++20 allows us to skip it, like this:
...
contents = std::move (output).str();
...
Use the more efficient version if compiling with -std=c++20.
Tested on x86_64-linux.
---
gdb/source-cache.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 77b357cb42b..7ef54411c69 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -252,7 +252,11 @@ source_cache::ensure (struct symtab *s)
std::istringstream input (contents);
std::ostringstream output;
highlighter->highlight (input, output, lang_name, fullname);
+#if __cplusplus >= 202002L
+ contents = std::move (output).str();
+#else
contents = output.str ();
+#endif
already_styled = true;
}
catch (...)
base-commit: 4b41a55fe53ce2da4823ffa3a5b94bd09bf2ab0d
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/4] [gdb/cli] Factor out try_source_highlight
2023-10-12 14:47 [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
@ 2023-10-12 14:47 ` Tom de Vries
2023-10-12 14:47 ` [PATCH 3/4] [gdb/cli] Keep track of styling failures in source_cache Tom de Vries
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2023-10-12 14:47 UTC (permalink / raw)
To: gdb-patches
Function source_cache::ensure contains some code using the GNU
source-highlight library.
The code is a sizable chunk of the function, and contains conditional
compilation in a slightly convoluted way:
...
if (!already_styled)
#endif /* HAVE_SOURCE_HIGHLIGHT */
{
...
Fix this by factoring out the code into new function try_source_highlight,
such that:
- source_cache::ensure is easier to read, and
- the conditional compilation is at the level of the function body.
Tested on x86_64-linux.
---
gdb/source-cache.c | 93 +++++++++++++++++++++++++++-------------------
1 file changed, 54 insertions(+), 39 deletions(-)
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 7ef54411c69..186a4f73256 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -191,6 +191,58 @@ get_language_name (enum language lang)
#endif /* HAVE_SOURCE_HIGHLIGHT */
+/* Try to highlight CONTENTS from file FULLNAME in language LANG_NAME using
+ the GNU source-higlight library. Return true if highlighting
+ succeeded. */
+
+static bool
+try_source_highlight (std::string &contents, const char *lang_name,
+ const std::string &fullname)
+{
+#ifdef HAVE_SOURCE_HIGHLIGHT
+ if (lang_name == nullptr || !use_gnu_source_highlight)
+ return false;
+
+ /* 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;
+
+ bool styled = false;
+ try
+ {
+ if (highlighter == nullptr)
+ {
+ highlighter = new srchilite::SourceHighlight ("esc.outlang");
+ highlighter->setStyleFile ("esc.style");
+ }
+
+ std::istringstream input (contents);
+ std::ostringstream output;
+ highlighter->highlight (input, output, lang_name, fullname);
+#if __cplusplus >= 202002L
+ contents = std::move (output).str();
+#else
+ contents = output.str ();
+#endif
+ styled = true;
+ }
+ catch (...)
+ {
+ /* Source Highlight will throw an exception if
+ highlighting fails. One possible reason it can fail
+ is if the language is unknown -- which matters to gdb
+ because Rust support wasn't added until after 3.1.8.
+ Ignore exceptions here. */
+ }
+
+ return styled;
+#else
+ return false;
+#endif /* HAVE_SOURCE_HIGHLIGHT */
+}
+
/* See source-cache.h. */
bool
@@ -230,48 +282,11 @@ source_cache::ensure (struct symtab *s)
if (source_styling && gdb_stdout->can_emit_style_escape ())
{
-#ifdef HAVE_SOURCE_HIGHLIGHT
- bool already_styled = false;
const char *lang_name = get_language_name (s->language ());
- if (lang_name != nullptr && use_gnu_source_highlight)
- {
- /* 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;
-
- try
- {
- if (highlighter == nullptr)
- {
- highlighter = new srchilite::SourceHighlight ("esc.outlang");
- highlighter->setStyleFile ("esc.style");
- }
-
- std::istringstream input (contents);
- std::ostringstream output;
- highlighter->highlight (input, output, lang_name, fullname);
-#if __cplusplus >= 202002L
- contents = std::move (output).str();
-#else
- contents = output.str ();
-#endif
- already_styled = true;
- }
- catch (...)
- {
- /* Source Highlight will throw an exception if
- highlighting fails. One possible reason it can fail
- is if the language is unknown -- which matters to gdb
- because Rust support wasn't added until after 3.1.8.
- Ignore exceptions here and fall back to
- un-highlighted text. */
- }
- }
+ bool already_styled
+ = try_source_highlight (contents, lang_name, fullname);
if (!already_styled)
-#endif /* HAVE_SOURCE_HIGHLIGHT */
{
gdb::optional<std::string> ext_contents;
ext_contents = ext_lang_colorize (fullname, contents);
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/4] [gdb/cli] Keep track of styling failures in source_cache
2023-10-12 14:47 [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
2023-10-12 14:47 ` [PATCH 2/4] [gdb/cli] Factor out try_source_highlight Tom de Vries
@ 2023-10-12 14:47 ` Tom de Vries
2023-10-12 14:47 ` [PATCH 4/4] [gdb/cli] Allow source highlighting to be interrupted Tom de Vries
2023-10-13 12:21 ` [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2023-10-12 14:47 UTC (permalink / raw)
To: gdb-patches
In source_cache::ensure, keep track of which files failed to be styled, and
don't attempt to style them again in case the file dropped out of the cache.
Tested on x86_64-linux.
---
gdb/source-cache.c | 24 ++++++++++++++++++++++--
gdb/source-cache.h | 4 ++++
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 186a4f73256..c29e5f4a210 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -280,7 +280,8 @@ source_cache::ensure (struct symtab *s)
return false;
}
- if (source_styling && gdb_stdout->can_emit_style_escape ())
+ if (source_styling && gdb_stdout->can_emit_style_escape ()
+ && m_no_styling_files.count (fullname) == 0)
{
const char *lang_name = get_language_name (s->language ());
bool already_styled
@@ -291,7 +292,26 @@ source_cache::ensure (struct symtab *s)
gdb::optional<std::string> ext_contents;
ext_contents = ext_lang_colorize (fullname, contents);
if (ext_contents.has_value ())
- contents = std::move (*ext_contents);
+ {
+ contents = std::move (*ext_contents);
+ already_styled = true;
+ }
+ }
+
+ if (!already_styled)
+ {
+ /* Styling failed. Styling can fail for instance for these
+ reasons:
+ - the language is not supported.
+ - the language cannot not be auto-detected from the file name.
+ - no stylers available.
+
+ Since styling failed, don't try styling the file again after it
+ drops from the cache.
+
+ Note that clearing the source cache also clears
+ m_no_styling_files. */
+ m_no_styling_files.insert (fullname);
}
}
diff --git a/gdb/source-cache.h b/gdb/source-cache.h
index bec6598ff9f..f1d30b43ce0 100644
--- a/gdb/source-cache.h
+++ b/gdb/source-cache.h
@@ -66,6 +66,7 @@ class source_cache
{
m_source_map.clear ();
m_offset_cache.clear ();
+ m_no_styling_files.clear ();
}
private:
@@ -95,6 +96,9 @@ class source_cache
/* The file offset cache. The key is the full name of the source
file. */
std::unordered_map<std::string, std::vector<off_t>> m_offset_cache;
+
+ /* The list of files where styling failed. */
+ std::unordered_set<std::string> m_no_styling_files;
};
/* The global source cache. */
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 4/4] [gdb/cli] Allow source highlighting to be interrupted
2023-10-12 14:47 [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
2023-10-12 14:47 ` [PATCH 2/4] [gdb/cli] Factor out try_source_highlight Tom de Vries
2023-10-12 14:47 ` [PATCH 3/4] [gdb/cli] Keep track of styling failures in source_cache Tom de Vries
@ 2023-10-12 14:47 ` Tom de Vries
2023-10-13 12:21 ` [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2023-10-12 14:47 UTC (permalink / raw)
To: gdb-patches
In PR cli/30934, a problem is reported where gdb becomes unresponsive for
2m13s because the GNU source-highlight library is taking a lot of time to
annotate the source.
This is due to a problem in the library [1], for which I've posted a
patch [2], which brings down the annotation time to 3s.
However, GDB should not become unresponsive due to such a problem.
Fix this by installing a srchilite::HighlightEventListener that checks whether
^C was pressed, and if so abandons the highlighting by throwing an exception.
Abandoning the highlighting looks like this to the user:
...
$ gdb -q a.out -ex "b 56"
Reading symbols from a.out...
Breakpoint 1 at 0x400e2a: file test.cpp, line 56.
(gdb) r
Starting program: /data/vries/gdb/a.out
Breakpoint 1, Solution::numOfSubarrays (this=0x7fffffffdbcf, arr=...) at \
test.cpp:56
^C
56 return (int) t;
(gdb)
...
Note that line 56 still can be highlighted, just by pygments instead of
source-highlight.
It may or may not be a good idea to issue a warning that highlighting by
source-highlight was interrupted, I've left it out for now.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30934
[1] https://savannah.gnu.org/bugs/?64747
[2] https://savannah.gnu.org/patch/index.php?10400
---
gdb/source-cache.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index c29e5f4a210..3ce2ea9d779 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -37,6 +37,7 @@
#include <sstream>
#include <srchilite/sourcehighlight.h>
#include <srchilite/langmap.h>
+#include <srchilite/highlighteventlistener.h>
#endif
/* The number of source files we'll cache. */
@@ -189,6 +190,21 @@ get_language_name (enum language lang)
return nullptr;
}
+/* Class with notify function called during highlighting. */
+
+class gdb_highlight_event_listener : public srchilite::HighlightEventListener
+{
+public:
+ void notify(const srchilite::HighlightEvent &event) override
+ {
+ if (check_quit_flag ())
+ {
+ /* Got SIGINT, interrupt highlighting, it may take too long. */
+ throw_quit ("Interrupted");
+ }
+ }
+};
+
#endif /* HAVE_SOURCE_HIGHLIGHT */
/* Try to highlight CONTENTS from file FULLNAME in language LANG_NAME using
@@ -216,6 +232,9 @@ try_source_highlight (std::string &contents, const char *lang_name,
{
highlighter = new srchilite::SourceHighlight ("esc.outlang");
highlighter->setStyleFile ("esc.style");
+
+ static gdb_highlight_event_listener event_listener;
+ highlighter->setHighlightEventListener (&event_listener);
}
std::istringstream input (contents);
@@ -304,13 +323,16 @@ source_cache::ensure (struct symtab *s)
reasons:
- the language is not supported.
- the language cannot not be auto-detected from the file name.
+ - styling took too long and was interrupted by the user.
- no stylers available.
Since styling failed, don't try styling the file again after it
drops from the cache.
Note that clearing the source cache also clears
- m_no_styling_files. */
+ m_no_styling_files, so if styling took too long, and the user
+ interrupted it, and the source cache gets cleared, the user will
+ need to interrupt styling again. */
m_no_styling_files.insert (fullname);
}
}
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure
2023-10-12 14:47 [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
` (2 preceding siblings ...)
2023-10-12 14:47 ` [PATCH 4/4] [gdb/cli] Allow source highlighting to be interrupted Tom de Vries
@ 2023-10-13 12:21 ` Tom de Vries
3 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2023-10-13 12:21 UTC (permalink / raw)
To: gdb-patches
I've submitted a v2 series here (
https://sourceware.org/pipermail/gdb-patches/2023-October/203199.html ).
Thanks,
- Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-13 12:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-12 14:47 [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
2023-10-12 14:47 ` [PATCH 2/4] [gdb/cli] Factor out try_source_highlight Tom de Vries
2023-10-12 14:47 ` [PATCH 3/4] [gdb/cli] Keep track of styling failures in source_cache Tom de Vries
2023-10-12 14:47 ` [PATCH 4/4] [gdb/cli] Allow source highlighting to be interrupted Tom de Vries
2023-10-13 12:21 ` [PATCH 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).