perf, tools, stat: Force C numeric locale for CSV mode
PATCH 1/1 perf, tools, stat: Force C numeric locale for CSV mode
From: Andi Kleen <[email protected]>
Some locales print floating point numbers with a comma instead of a dot. This causes problems with CSV mode because it causes extra false CSV fields. Force the numeric locale to be always C in CSV mode.
Before:
$ LC_ALL=pl_PL.utf8 perf stat -x, true 0,399472,,task-clock,399472,100,00 <---- extra bogus field ...
After:
$ LC_ALL=pl_PL.utf8 ./obj-perf/perf stat -x, true 0.338422,,task-clock,338422,100.00
Originally reported in https://github.com/andikleen/pmu-tools/issues/43
--- tools/perf/builtin-stat.c | 1 + 1 file changed, 1 insertion(+)
tools/perf/builtin-stat.c +1 -0
@@ -1826,6 +1826,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused) csv_output = true;if (!strcmp (csv_sep,"\\t" )) csv_sep ="\t" ; +setlocale (LC_NUMERIC ,"C" ); }else csv_sep =DEFAULT_SEPARATOR ;
Signature · 1 line
2.4.3
Quoted 12 lines · Andi Kleen
From: Andi Kleen <[email protected]>
Some locales print floating point numbers with a comma instead of a dot. This causes problems with CSV mode because it causes extra false CSV fields. Force the numeric locale to be always C in CSV mode.
Before:
$ LC_ALL=pl_PL.utf8 perf stat -x, true 0,399472,,task-clock,399472,100,00 <---- extra bogus field ...
Quoted 7 lines
After:
$ LC_ALL=pl_PL.utf8 ./obj-perf/perf stat -x, true 0.338422,,task-clock,338422,100.00Originally reported in https://github.com/andikleen/pmu-tools/issues/43
Signed-off-by Andi Kleen <[email protected]>
thanks,
jirka
Arnaldo Carvalho de Melo
maintainerEm Tue, Jan 05, 2016 at 10:05:01PM +0100, Jiri Olsa escreveu:
Quoted 17 lines · Andi Kleen
Quoted 16 lines
From: Andi Kleen <[email protected]>
Some locales print floating point numbers with a comma instead of a dot. This causes problems with CSV mode because it causes extra false CSV fields. Force the numeric locale to be always C in CSV mode.
Before:
$ LC_ALL=pl_PL.utf8 perf stat -x, true 0,399472,,task-clock,399472,100,00 <---- extra bogus field ...After:
$ LC_ALL=pl_PL.utf8 ./obj-perf/perf stat -x, true 0.338422,,task-clock,338422,100.00
Quoted 5 lines
Quoted 3 lines
Originally reported in https://github.com/andikleen/pmu-tools/issues/43
Signed-off-by Andi Kleen <[email protected]>Acked-by Jiri Olsa <[email protected]>
I wonder what is that other tools do when stumbling on this, i.e. some other tool output that produces values that have the CSV character in it...
Completely disabling the configured locale seems too harsh to me, aren't people used to changing the csv char via some option like we have in 'perf stat':
-x, --field-separator
when changing the locale from the default 'C' one? Hey, you even used it above, but you chose a CSV char that is used in this locale, oops ;-)
- Arnaldo
Quoted 20 lines · Arnaldo Carvalho de Melo
Em Tue, Jan 05, 2016 at 10:05:01PM +0100, Jiri Olsa escreveu:
Quoted 19 lines · Andi Kleen
Quoted 18 lines
From: Andi Kleen <[email protected]>
Some locales print floating point numbers with a comma instead of a dot. This causes problems with CSV mode because it causes extra false CSV fields. Force the numeric locale to be always C in CSV mode.
Before:
$ LC_ALL=pl_PL.utf8 perf stat -x, true 0,399472,,task-clock,399472,100,00 <---- extra bogus field ...After:
$ LC_ALL=pl_PL.utf8 ./obj-perf/perf stat -x, true 0.338422,,task-clock,338422,100.00Originally reported in https://github.com/andikleen/pmu-tools/issues/43
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Andi Kleen <[email protected]>Acked-by Jiri Olsa <[email protected]>I wonder what is that other tools do when stumbling on this, i.e. some other tool output that produces values that have the CSV character in it...
Proper CSV supports escaping the separator by putting the whole field into quotes. Unfortunately perf stat doesn't output proper CSV, the event fields with commas are not quoted.
I usually work around it by using -x\; instead
But the , problem should be still fixed.
Quoted 9 lines
Completely disabling the configured locale seems too harsh to me, aren't people used to changing the csv char via some option like we have in 'perf stat':
-x, --field-separatorwhen changing the locale from the default 'C' one? Hey, you even used it above, but you chose a CSV char that is used in this locale, oops ;-)
It's just for numbers (LC_NUMERIC), everything else is still localized.
-Andi
Arnaldo Carvalho de Melo
maintainerEm Tue, Jan 05, 2016 at 10:28:39PM +0100, Andi Kleen escreveu:
Quoted 25 lines · Arnaldo Carvalho de Melo
Quoted 24 lines
Em Tue, Jan 05, 2016 at 10:05:01PM +0100, Jiri Olsa escreveu:
Quoted 22 lines · Andi Kleen
Quoted 12 lines
From: Andi Kleen <[email protected]>
Some locales print floating point numbers with a comma instead of a dot. This causes problems with CSV mode because it causes extra false CSV fields. Force the numeric locale to be always C in CSV mode.
Before:
$ LC_ALL=pl_PL.utf8 perf stat -x, true 0,399472,,task-clock,399472,100,00 <---- extra bogus field ...Quoted 7 lines
After:
$ LC_ALL=pl_PL.utf8 ./obj-perf/perf stat -x, true 0.338422,,task-clock,338422,100.00Originally reported in https://github.com/andikleen/pmu-tools/issues/43
Signed-off-by Andi Kleen <[email protected]>Acked-by Jiri Olsa <[email protected]>
Quoted 7 lines
Quoted 3 lines
I wonder what is that other tools do when stumbling on this, i.e. some other tool output that produces values that have the CSV character in it...
Proper CSV supports escaping the separator by putting the whole field into quotes. Unfortunately perf stat doesn't output proper CSV, the event fields with commas are not quoted.
Right, there is even an RFC for CSV, and for a decade already :-)
https://tools.ietf.org/html/rfc4180
Quoted 3 lines
I usually work around it by using -x\; instead
But the , problem should be still fixed.
Humm, what is the problem then of doing, for example in my case, with a LC_ALL=pt_BR, that uses commans as the decimal separator:
LC_ALL=C ./obj-perf/perf stat -x, true
I.e. disabling it using the existing shell mecanism?
[root@zoo ~]# export LC_ALL=pt_BR
[root@zoo ~]# perf stat -e cycles -x, usleep 1
1068190,,cycles,1007049,100,00
Bad, but its a side effect of needing to use a locale that uses the CSV separator in the decimal separator, disabling it for commands where I want that CSV separator does the trick:
[root@zoo ~]# LC_ALL= perf stat -e cycles -x, usleep 1 895081,,cycles,840687,100.00
[root@zoo ~]#
It is an inconvenience, yeah, having to prefix that for tools where I want to use the comma for the CSV separator, but disabling LC_NUMERIC, albeit better than my what I misparsed at first (forcing all locale to 'C') still looks too harsh :-\
Using -x\; looks sane and shorter tho, perhaps even -x:, to save one extra char.
- Arnaldo
Quoted 12 lines
Quoted 8 lines
Completely disabling the configured locale seems too harsh to me, aren't people used to changing the csv char via some option like we have in 'perf stat':
-x, --field-separatorwhen changing the locale from the default 'C' one? Hey, you even used it above, but you chose a CSV char that is used in this locale, oops ;-)
It's just for numbers (
LC_NUMERIC), everything else is still localized.-Andi
Quoted 6 lines
Quoted 3 lines
I usually work around it by using -x\; instead
But the , problem should be still fixed.
Humm, what is the problem then of doing, for example in my case, with a LC_ALL=pt_BR, that uses commans as the decimal separator:
It's user unfriendly and unobvious. Also you end up with subtly broken files,. And it would also change the locale of the measured program which may not be intended.
Plus the floating point values with comma cannot be parsed by programs that don't know your locale (that was the problem with pmu-tools)
Quoted 2 lines
Using -x\; looks sane and shorter tho, perhaps even -x:, to save one extra char.
Even with that there is the problem that you end up with numbers that cannot be parsed by locale unaware programs.
CSV is intended for other programs so it shouldn't be messed up like this.
-Andi