diff mbox series

[v2,1/4,gdb/cli] Skip string copy in source_cache::ensure

Message ID 20231013121953.25917-2-tdevries@suse.de
State Superseded
Headers
Series Allow source highlighting to be interrupted |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom de Vries Oct. 13, 2023, 12:19 p.m. UTC 
  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(+)

Comments

Lancelot SIX Oct. 13, 2023, 4:34 p.m. UTC | #1 
Hi Tom,

On Fri, Oct 13, 2023 at 02:19:50PM +0200, Tom de Vries wrote:
> 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();
> ...

Looking at cppreference[1], it suggests that this new C++ overload for
str should be equivalent to

  content = std::move (*output.rbuf ()).str ();

Should we use this construct instead regardless of C++'s version and
avoid the #ifdef?  That being said, I find the C++20 version easier to
read, so it's up to you.

Best,
Lancelot.

[1] https://en.cppreference.com/w/cpp/io/basic_ostringstream/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 (...)
> -- 
> 2.35.3
>
Tom de Vries Oct. 16, 2023, 7:40 a.m. UTC | #2 
On 10/13/23 18:34, Lancelot SIX wrote:
> Hi Tom,
> 
> On Fri, Oct 13, 2023 at 02:19:50PM +0200, Tom de Vries wrote:
>> 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();
>> ...
> 
> Looking at cppreference[1], it suggests that this new C++ overload for
> str should be equivalent to
> 
>    content = std::move (*output.rbuf ()).str ();
> 
> Should we use this construct instead regardless of C++'s version and
> avoid the #ifdef?  That being said, I find the C++20 version easier to
> read, so it's up to you.
> 

I've played around with this a bit, and found out that I can also use 
"contents = std::move (output).str();" in c++11.  So I've just dropped 
the ifdef.

Thanks,
- Tom

> Best,
> Lancelot.
> 
> [1] https://en.cppreference.com/w/cpp/io/basic_ostringstream/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 (...)
>> -- 
>> 2.35.3
>>
diff mbox series

Patch

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 (...)