Re: [Maria-developers] 203aa38f9b6: MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json instead - refactoring needed
Hi Serg,
On Thu, Jul 29, 2021 at 9:57 AM Sergei Golubchik wrote:
> Hi, Anel!
>
> On Jul 29, Anel Husakovic wrote:
> > Hi Serg,
> > thanks for your review.
> >
> > > > +
> > > > +my $gcc_version= `gcc -dumpversion`;
> > > > +$gcc_version=~ s/(\d).*$/$1/;
> > >
> > > you don't need this second line, I suspect
> > >
> > It is not unique for all versions, that's why I used it:
> >
> > $ for i in 5 6 7 9 10; do $(/usr/bin/gcc-$i -dumpversion); done
> > 5.5.0: command not found
> > 6.5.0: command not found
> > 7: command not found
> > 9: command not found
> > 10: command not found
>
> I don't understand what you mean or what you're doing here.
>
I tried to show what is the output of `dumpversion` for various versions of
gcc and why regex for the first digit is needed.
> > > and write `my $fname` here
> > >
> > > > + for my $line (@{$file->{lines}}){
> > > > +$cov{$fname}->{$line->{line_number}}= $line->{count};
> > >
> > > shouldn't it be += $line->{count} ?
> > > The point is to accumulate all counters, see how it's done for non-json
> > > intermediate format.
> > >
> > > > + }
> > > > +}
> > > >}
> > > > }
> > >
> > Have seen and I'm not sure how it accumulates since per line
> "line_number"
> > value is different (I haven't seen 'lcount' to have the same
> > line_number), so the hash is different and += doesn't take effect.
>
> it's for the case like, say, THD::exit_cond() - an inline method defined
> in sql_class.h. It's included in many .cc files and many *.gcov files
> will have lcount data for it.
>
> Make sense. Thanks.
Applied in new patch: 85aa3f8cf66fd76addaa615dca507f
Regards,
Anel
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] 203aa38f9b6: MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json instead - refactoring needed
Hi, Anel!
On Jul 29, Anel Husakovic wrote:
> Hi Serg,
> thanks for your review.
>
> > > +
> > > +my $gcc_version= `gcc -dumpversion`;
> > > +$gcc_version=~ s/(\d).*$/$1/;
> >
> > you don't need this second line, I suspect
> >
> It is not unique for all versions, that's why I used it:
>
> $ for i in 5 6 7 9 10; do $(/usr/bin/gcc-$i -dumpversion); done
> 5.5.0: command not found
> 6.5.0: command not found
> 7: command not found
> 9: command not found
> 10: command not found
I don't understand what you mean or what you're doing here.
> > and write `my $fname` here
> >
> > > + for my $line (@{$file->{lines}}){
> > > +$cov{$fname}->{$line->{line_number}}= $line->{count};
> >
> > shouldn't it be += $line->{count} ?
> > The point is to accumulate all counters, see how it's done for non-json
> > intermediate format.
> >
> > > + }
> > > +}
> > >}
> > > }
> >
> Have seen and I'm not sure how it accumulates since per line "line_number"
> value is different (I haven't seen 'lcount' to have the same
> line_number), so the hash is different and += doesn't take effect.
it's for the case like, say, THD::exit_cond() - an inline method defined
in sql_class.h. It's included in many .cc files and many *.gcov files
will have lcount data for it.
Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] 203aa38f9b6: MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json instead - refactoring needed
Hi Serg,
thanks for your review.
> > +
> > +my $gcc_version= `gcc -dumpversion`;
> > +$gcc_version=~ s/(\d).*$/$1/;
>
> you don't need this second line, I suspect
>
>
It is not unique for all versions, that's why I used it:
$ for i in 5 6 7 9 10; do $(/usr/bin/gcc-$i -dumpversion); done
5.5.0: command not found
6.5.0: command not found
7: command not found
9: command not found
10: command not found
> and write `my $fname` here
>
> > + for my $line (@{$file->{lines}}){
> > +$cov{$fname}->{$line->{line_number}}= $line->{count};
>
> shouldn't it be += $line->{count} ?
> The point is to accumulate all counters, see how it's done for non-json
> intermediate format.
>
> > + }
> > +}
> >}
> > }
> >
>
Have seen and I'm not sure how it accumulates since per line "line_number"
value is different (I haven't seen 'lcount' to have the same
line_number), so the hash is different and += doesn't take effect.
Confirmed that I'm getting the same result with/without += for both
versions ( since line number is a function of the counter (one to one
relationship) ?)
Maybe to remove += from gcc < 9.1 ?
gcov 7.5:
$ cat sql_show.cc.gcda##field.h.gcov
lcount:847,20 # this line : line_number=847, count = 20
lcount:848,20 # not the same as this one, line_number=848, 20
lcount:851,0
lcount:852,0
lcount:878,0
lcount:981,1181
gcov 9.4:
$ cat
"lines" : [
{
"unexecuted_block" : false,
"function_name" : "sum",
"count" : 1,
"branches" : [],
"line_number" : 3 # this line
},
{
"function_name" : "sum",
"unexecuted_block" : false,
"count" : 1,
"line_number" : 5, # is not the same as this line
"branches" : []
}
],
However I'm confirming that there is different result in counter for the
same test and I'm suspecting it is the result of generated parsing between
gcov versions:
gcov 7.5 ( counter = 88):
*
dgcov sql/sql_show.cc
*
@@ +6601,7 @@ static int get_check_constraints_record(THD *thd, TABLE_LIST
*tables,
: 6601: }
: 6602:#endif
: 6603: Virtual_column_info *check=
tables->table->check_constraints[i];
88: 6605: table->field[0]->store(STRING_WITH_LEN("definition"),
system_charset_info);
: 6606: table->field[3]->store(check->name.str,
check->name.length,
: 6607: system_charset_info);
gcov 9.4 ( counter = 44):
*
dgcov sql/sql_show.cc
*
@@ +6601,7 @@ static int get_check_constraints_record(THD *thd, TABLE_LIST
*tables,
: 6601: }
: 6602:#endif
: 6603: Virtual_column_info *check=
tables->table->check_constraints[i];
44: 6605: table->field[0]->store(STRING_WITH_LEN("definition"),
system_charset_info);
: 6606: table->field[3]->store(check->name.str,
check->name.length,
: 6607: system_charset_info);
Regards,
Anel
> _
> Mailing list: https://launchpad.net/~maria-developers
> Post to : maria-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~maria-developers
> More help : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] 203aa38f9b6: MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json instead - refactoring needed
Hi, Anel!
On Jul 28, Anel Husakovic wrote:
> revision-id: 203aa38f9b6 (mariadb-10.2.39-96-g203aa38f9b6)
> parent(s): afe00bb7cce
> author: Anel Husakovic
> committer: Anel Husakovic
> timestamp: 2021-07-27 16:09:41 +0200
> message:
>
> MDEV-26102: gcov 9.1 doesn't support intermediate format any more, but json
> instead - refactoring needed
I've renamed the MDEV to have a shorter title, use the new one in your
next commit
>
> Reviewed by: s...@mariadb.com
that's not right, I haven't reviewed it yet :)
> diff --git a/mysql-test/dgcov.pl b/mysql-test/dgcov.pl
> index 2c00c64d1ff..e2aadec33e0 100755
> --- a/mysql-test/dgcov.pl
> +++ b/mysql-test/dgcov.pl
> @@ -25,6 +25,8 @@ use warnings;
> use Getopt::Long;
> use File::Find;
> use Cwd qw/realpath/;
> +use IO::Uncompress::Gunzip qw(gunzip $GunzipError);
> +use JSON::PP;
move this down, dcgov should not try to load these modules for for gcc < 9
> my $opt_verbose=0;
> my $opt_generate;
> @@ -32,6 +34,7 @@ my $opt_help;
> my $opt_purge;
> my $opt_only_gcov;
> my $opt_skip_gcov;
> +my $fname;
and this
>
> my %cov;
> my $file_no=0;
> @@ -62,12 +65,15 @@ my $res;
> my $cmd;
> if ($opt_purge)
> {
> - $cmd= "find . -name '*.da' -o -name '*.gcda' -o -name '*.gcov' -o ".
> + $cmd= "find . -name '*.da' -o -name '*.gcda*' -o -name '*.gcov' -o ".
> "-name '*.dgcov' | grep -v 'README\.gcov' | xargs rm -f ''";
>logv "Running: $cmd";
>system($cmd)==0 or die "system($cmd): $? $!";
>exit 0;
> }
> +
> +my $gcc_version= `gcc -dumpversion`;
> +$gcc_version=~ s/(\d).*$/$1/;
you don't need this second line, I suspect
>
> find(\_one_file, $root);
> find(\_coverage, $root) if $opt_generate;
> @@ -162,6 +168,7 @@ sub gcov_one_file {
>}
>
># now, read the generated file
> + if ($gcc_version <9){
>for my $gcov_file (<$_*.gcov>) {
> open FH, '<', "$gcov_file" or die "open(<$gcov_file): $!";
> my $fname;
> @@ -183,6 +189,18 @@ sub gcov_one_file {
>$cov{$fname}->{$1}+=$2;
> }
> close(FH);
> +}
> + }
> + else {
put both `use` lines here
> +my $gcov_file_json;
> +gunzip "$_.gcov.json.gz" => \$gcov_file_json or die
> "gunzip($_.gcov.json.gz): $GunzipError";
> +my $obj= decode_json $gcov_file_json;
> +for my $file (@{$obj->{files}}) {
> + $fname= $file->{file};
and write `my $fname` here
> + for my $line (@{$file->{lines}}){
> +$cov{$fname}->{$line->{line_number}}= $line->{count};
shouldn't it be += $line->{count} ?
The point is to accumulate all counters, see how it's done for non-json
intermediate format.
> + }
> +}
>}
> }
>
>
Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
