Re: Regression for GDB global --statistics

  Jan Kratochvil writes:
 > On Fri, 15 Mar 2013 23:29:09 +0100, Doug Evans wrote:
 > > This patch adds a new option to display some simple symtab stats
 > > akin to how "mt time|space 1" work.  For consistency with the
 > > rest of gdb I named it:
 > > 
 > > maint set per-command symtab on|off
 > > maint show per-command symtab
 > 
 > bd712aed2f88ab824d403c55a212c2be3f41a335 is the first bad commit
 > commit bd712aed2f88ab824d403c55a212c2be3f41a335
 > Author: Doug Evans <dje@google.com>
 > Date:   Thu Mar 21 17:37:30 2013 +0000
 >         New commands "mt set per-command {space,time,symtab} {on,off}".
 > 
 > before (gdb-7.6):
 > gdb -nx -q -statistics  </dev/null
 > Startup time: 0.031000 (cpu), 0.033828 (wall)
 > Space used: 4005888 (+4005888 during startup)
 > (gdb) quit
 > 
 > after (gdb-7.7+):
 > gdb -nx -q -statistics  </dev/null
 > (gdb) quit
 > 
 > Commands stats work now but no longer the global stats.  Command stats look
 > differently ("for this command" vs. "during startup"):
 > echo echo|gdb -nx -q -statistics 
 > (gdb) Command execution time: 0.000000 (cpu), 0.000008 (wall)
 > Space used: 4206592 (+0 for this command)
 > (gdb) quit
 > 
 > It has been found by Miroslav Franc.

Thanks.

Here's a patch.

2014-07-09  Doug Evans  <dje@google.com>

	* maint.c (count_symtabs_and_blocks): Handle NULL
	current_program_space.
	(report_command_stats): Check global enabled flag in addition to
	recorded enabled flag.
	(make_command_stats_cleanup): Handle msg_type == 0, startup.

	testsuite/
	* gdb.base/maint.exp: Update testing of per-command stats.
  

Hi Doug,

On 07/09/2014 10:10 PM, Doug Evans wrote:
> Here's a patch.
> 
> 2014-07-09  Doug Evans  <dje@google.com>
> 
> 	* maint.c (count_symtabs_and_blocks): Handle NULL
> 	current_program_space.

It's not obvious to me how this can be NULL, given we initialize
it so early.  If there's a good reason, could you please add
a comment here mentioning it?

Thanks,
Pedro Alves
  

On Wed, 09 Jul 2014 23:10:56 +0200, Doug Evans wrote:
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -130,10 +130,11 @@ gdb_expect  {
>  # tests here!!
>  gdb_test_no_output "maint check-symtabs"
>  
> +# Test per-command stats.
>  gdb_test_no_output "maint set per-command on"
> -
> -gdb_test "maint set per-command off" \
> +gdb_test "pwd" \
>      "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
> +gdb_test_no_output "maint set per-command off"

Is it needed to change this behavior?  It may break some user scripts working
fine with previous GDB stable releases.

Besides that the startup statistics regression is not tested here but I can
post then a testcase for it.


Thanks,
Jan
  

On Fri, Jul 11, 2014 at 2:25 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 09 Jul 2014 23:10:56 +0200, Doug Evans wrote:
>> --- a/gdb/testsuite/gdb.base/maint.exp
>> +++ b/gdb/testsuite/gdb.base/maint.exp
>> @@ -130,10 +130,11 @@ gdb_expect  {
>>  # tests here!!
>>  gdb_test_no_output "maint check-symtabs"
>>
>> +# Test per-command stats.
>>  gdb_test_no_output "maint set per-command on"
>> -
>> -gdb_test "maint set per-command off" \
>> +gdb_test "pwd" \
>>      "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
>> +gdb_test_no_output "maint set per-command off"
>
> Is it needed to change this behavior?  It may break some user scripts working
> fine with previous GDB stable releases.

Change in behaviour of something related to a maintenance command?
Or were you referring to something else?

One problem I wish to avoid is not doing anything expensive unnecessarily.
And here that translates to not computing the values of statistics
from before a command executes (so that one can then print the delta
after it completes) if said statistics will not be printed.  That then
translates to any turning on of a statistics-reporting command (mt set
per-command foo) will not take effect until the next command.  How
will that be a problem?

> Besides that the startup statistics regression is not tested here but I can
> post then a testcase for it.

As you wish.  Thanks.