All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perfc: Print a system time in a convenient format
@ 2018-09-11  6:53 Andrii Anisov
  2018-09-11  8:27 ` Jan Beulich
  0 siblings, 1 reply; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11  6:53 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Andrii Anisov, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, xen-devel, Wei Liu

From: Andrii Anisov <andrii_anisov@epam.com>

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/common/perfc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/common/perfc.c b/xen/common/perfc.c
index 0675677..0abd977 100644
--- a/xen/common/perfc.c
+++ b/xen/common/perfc.c
@@ -33,8 +33,7 @@ void perfc_printall(unsigned char key)
     unsigned int i, j;
     s_time_t now = NOW();
 
-    printk("Xen performance counters SHOW  (now = 0x%08X:%08X)\n",
-           (u32)(now>>32), (u32)now);
+    printk("Xen performance counters SHOW  (now = %"PRI_stime")\n", now);
 
     for ( i = j = 0; i < NR_PERFCTRS; i++ )
     {
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  6:53 [PATCH] perfc: Print a system time in a convenient format Andrii Anisov
@ 2018-09-11  8:27 ` Jan Beulich
  2018-09-11  8:50   ` Andrii Anisov
  0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  8:27 UTC (permalink / raw)
  To: andrii.anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
	Julien Grall, xen-devel, andrii_anisov

>>> On 11.09.18 at 08:53, <andrii.anisov@gmail.com> wrote:
> --- a/xen/common/perfc.c
> +++ b/xen/common/perfc.c
> @@ -33,8 +33,7 @@ void perfc_printall(unsigned char key)
>      unsigned int i, j;
>      s_time_t now = NOW();
>  
> -    printk("Xen performance counters SHOW  (now = 0x%08X:%08X)\n",
> -           (u32)(now>>32), (u32)now);
> +    printk("Xen performance counters SHOW  (now = %"PRI_stime")\n", now);

NAK, for two reasons: I'm not of the opinion that reading a 15 or more
digit decimal number without any separators is any easier than the
current format. And then if you/we were to change this, then please
uniformly everywhere - there's even a second instance right in this
same file you change, not to speak of similar ones elsewhere.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  8:27 ` Jan Beulich
@ 2018-09-11  8:50   ` Andrii Anisov
  2018-09-11  9:10     ` Jan Beulich
  2018-09-11  9:24     ` George Dunlap
  0 siblings, 2 replies; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11  8:50 UTC (permalink / raw)
  To: Jan Beulich, andrii.anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
	Julien Grall, xen-devel

Hello Jan,


On 11.09.18 11:27, Jan Beulich wrote:
> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
> digit decimal number without any separators is any easier than the
> current format.
It's quite subjective. IMHO timestamps measured in ns easier to 
understand in decimals rather than in separated 32-bit hex-es. No matter 
how many digital number they have.
Even post processing of perfc output is easier in case of decimal 
timestamps. You should not parse hexes and odd separators to calculate 
the time elapsed between two samples.

>   And then if you/we were to change this, then please
> uniformly everywhere - there's even a second instance right in this
> same file you change, not to speak of similar ones elsewhere
I'll check through this file.

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  8:50   ` Andrii Anisov
@ 2018-09-11  9:10     ` Jan Beulich
  2018-09-11  9:15       ` Andrew Cooper
  2018-09-11  9:19       ` Andrii Anisov
  2018-09-11  9:24     ` George Dunlap
  1 sibling, 2 replies; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  9:10 UTC (permalink / raw)
  To: andrii_anisov, andrii.anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
	Julien Grall, xen-devel

>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
> On 11.09.18 11:27, Jan Beulich wrote:
>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>> digit decimal number without any separators is any easier than the
>> current format.
> It's quite subjective. IMHO timestamps measured in ns easier to 
> understand in decimals rather than in separated 32-bit hex-es. No matter 
> how many digital number they have.

It is subjective, yes, but in such a case you even more so need to
demonstrate a change is an overall improvement.

> Even post processing of perfc output is easier in case of decimal 
> timestamps. You should not parse hexes and odd separators to calculate 
> the time elapsed between two samples.

Post processing usually uses scrips for parsing - I don't think it's
overly complicated to have a script convert the number into
basically any format you want.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:10     ` Jan Beulich
@ 2018-09-11  9:15       ` Andrew Cooper
  2018-09-11  9:18         ` Jan Beulich
  2018-09-11  9:19       ` Andrii Anisov
  1 sibling, 1 reply; 21+ messages in thread
From: Andrew Cooper @ 2018-09-11  9:15 UTC (permalink / raw)
  To: Jan Beulich, andrii_anisov, andrii.anisov
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Tim Deegan, Ian Jackson, xen-devel, Julien Grall,
	xen-devel

On 11/09/18 10:10, Jan Beulich wrote:
>>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
>> On 11.09.18 11:27, Jan Beulich wrote:
>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>> digit decimal number without any separators is any easier than the
>>> current format.
>> It's quite subjective. IMHO timestamps measured in ns easier to 
>> understand in decimals rather than in separated 32-bit hex-es. No matter 
>> how many digital number they have.
> It is subjective, yes, but in such a case you even more so need to
> demonstrate a change is an overall improvement.
>
>> Even post processing of perfc output is easier in case of decimal 
>> timestamps. You should not parse hexes and odd separators to calculate 
>> the time elapsed between two samples.
> Post processing usually uses scrips for parsing - I don't think it's
> overly complicated to have a script convert the number into
> basically any format you want.

Right, but this particular mis-pattern wants fixing.  It is simply
confusing to have a single number formatted with a colon inbetween.

andrewcoop@andrewcoop:/local/xen.git/xen$ git grep -i "now>>32"
arch/x86/numa.c:381:           (u32)(now>>32), (u32)now);
common/keyhandler.c:258:           (u32)(now>>32), (u32)now);
common/page_alloc.c:2424:           (u32)(now>>32), (u32)now);
common/perfc.c:37:           (u32)(now>>32), (u32)now);
common/perfc.c:126:               (u32)(now>>32), (u32)now);
common/spinlock.c:363:        "total = %08X:%08X)\n", (u32)(now>>32),
(u32)now,
common/spinlock.c:383:            (u32)(now>>32), (u32)now);

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:15       ` Andrew Cooper
@ 2018-09-11  9:18         ` Jan Beulich
  2018-09-11  9:20           ` Andrew Cooper
                             ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  9:18 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Stefano Stabellini, andrii_anisov, Konrad Rzeszutek Wilk,
	George Dunlap, Tim Deegan, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, Wei Liu

>>> On 11.09.18 at 11:15, <andrew.cooper3@citrix.com> wrote:
> On 11/09/18 10:10, Jan Beulich wrote:
>>>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
>>> On 11.09.18 11:27, Jan Beulich wrote:
>>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>>> digit decimal number without any separators is any easier than the
>>>> current format.
>>> It's quite subjective. IMHO timestamps measured in ns easier to 
>>> understand in decimals rather than in separated 32-bit hex-es. No matter 
>>> how many digital number they have.
>> It is subjective, yes, but in such a case you even more so need to
>> demonstrate a change is an overall improvement.
>>
>>> Even post processing of perfc output is easier in case of decimal 
>>> timestamps. You should not parse hexes and odd separators to calculate 
>>> the time elapsed between two samples.
>> Post processing usually uses scrips for parsing - I don't think it's
>> overly complicated to have a script convert the number into
>> basically any format you want.
> 
> Right, but this particular mis-pattern wants fixing.  It is simply
> confusing to have a single number formatted with a colon inbetween.

What other separator do you suggest? I consider it quite helpful
to have one when there are more than about 10 digits. (I'd also
be fine, btw, to have the time printed in decimal, but then
perhaps with ms granularity and 6 digits for the fractional part.)

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:10     ` Jan Beulich
  2018-09-11  9:15       ` Andrew Cooper
@ 2018-09-11  9:19       ` Andrii Anisov
  1 sibling, 0 replies; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11  9:19 UTC (permalink / raw)
  To: Jan Beulich, andrii.anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
	Julien Grall, xen-devel


On 11.09.18 12:10, Jan Beulich wrote:
> It is subjective, yes, but in such a case you even more so need to
> demonstrate a change is an overall improvement.
I would phrase it as "printing timestapms in timestamp format".

-- 

*Andrii Anisov*


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:18         ` Jan Beulich
@ 2018-09-11  9:20           ` Andrew Cooper
  2018-09-11  9:28             ` Jan Beulich
  2018-09-11  9:21           ` Andrii Anisov
  2018-09-11  9:28           ` George Dunlap
  2 siblings, 1 reply; 21+ messages in thread
From: Andrew Cooper @ 2018-09-11  9:20 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, andrii_anisov, Konrad Rzeszutek Wilk,
	George Dunlap, Tim Deegan, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, Wei Liu

On 11/09/18 10:18, Jan Beulich wrote:
>>>> On 11.09.18 at 11:15, <andrew.cooper3@citrix.com> wrote:
>> On 11/09/18 10:10, Jan Beulich wrote:
>>>>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
>>>> On 11.09.18 11:27, Jan Beulich wrote:
>>>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>>>> digit decimal number without any separators is any easier than the
>>>>> current format.
>>>> It's quite subjective. IMHO timestamps measured in ns easier to 
>>>> understand in decimals rather than in separated 32-bit hex-es. No matter 
>>>> how many digital number they have.
>>> It is subjective, yes, but in such a case you even more so need to
>>> demonstrate a change is an overall improvement.
>>>
>>>> Even post processing of perfc output is easier in case of decimal 
>>>> timestamps. You should not parse hexes and odd separators to calculate 
>>>> the time elapsed between two samples.
>>> Post processing usually uses scrips for parsing - I don't think it's
>>> overly complicated to have a script convert the number into
>>> basically any format you want.
>> Right, but this particular mis-pattern wants fixing.  It is simply
>> confusing to have a single number formatted with a colon inbetween.
> What other separator do you suggest? I consider it quite helpful
> to have one when there are more than about 10 digits. (I'd also
> be fine, btw, to have the time printed in decimal, but then
> perhaps with ms granularity and 6 digits for the fractional part.)

It is currently more common across the base to use the format proposed
by Andrii.

For consistently alone, that format should be followed, or *everything*
should be changed.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:18         ` Jan Beulich
  2018-09-11  9:20           ` Andrew Cooper
@ 2018-09-11  9:21           ` Andrii Anisov
  2018-09-11  9:30             ` Jan Beulich
  2018-09-11  9:28           ` George Dunlap
  2 siblings, 1 reply; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11  9:21 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Tim Deegan, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel


On 11.09.18 12:18, Jan Beulich wrote:
> What other separator do you suggest? I consider it quite helpful
> to have one when there are more than about 10 digits. (I'd also
> be fine, btw, to have the time printed in decimal, but then
> perhaps with ms granularity and 6 digits for the fractional part.)
Should we think about replacing PRI_stime?

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  8:50   ` Andrii Anisov
  2018-09-11  9:10     ` Jan Beulich
@ 2018-09-11  9:24     ` George Dunlap
  2018-09-11  9:33       ` Jan Beulich
  1 sibling, 1 reply; 21+ messages in thread
From: George Dunlap @ 2018-09-11  9:24 UTC (permalink / raw)
  To: Andrii Anisov, Jan Beulich, andrii.anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
	Julien Grall, xen-devel

On 09/11/2018 09:50 AM, Andrii Anisov wrote:
> Hello Jan,
> 
> 
> On 11.09.18 11:27, Jan Beulich wrote:
>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>> digit decimal number without any separators is any easier than the
>> current format.
> It's quite subjective. IMHO timestamps measured in ns easier to
> understand in decimals rather than in separated 32-bit hex-es. No matter
> how many digital number they have.
> Even post processing of perfc output is easier in case of decimal
> timestamps. You should not parse hexes and odd separators to calculate
> the time elapsed between two samples.

I agree with this.  Printing in hexadecimal is useful if you need to see
the bitwise structure of the content; that's of zero use when it comes
to timestamps.  On the contrary, you almost always want to be able to do
a subtraction in your head and get at least an order-of-magnitude
difference between the times, something which is far too difficult in hex.

The fact that we even have PRI_stime is evidence that the developer
community as a whole agrees.

>>   And then if you/we were to change this, then please
>> uniformly everywhere - there's even a second instance right in this
>> same file you change, not to speak of similar ones elsewhere

Presumably the fact that we have PRI_stime defined means that *most*
places already use this format, and the places with `:` in the middle
are outliers.  It's perfectly acceptable to change them one by one;
changing them all should only be a suggestion.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:18         ` Jan Beulich
  2018-09-11  9:20           ` Andrew Cooper
  2018-09-11  9:21           ` Andrii Anisov
@ 2018-09-11  9:28           ` George Dunlap
  2018-09-11  9:37             ` Jan Beulich
  2 siblings, 1 reply; 21+ messages in thread
From: George Dunlap @ 2018-09-11  9:28 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper
  Cc: Stefano Stabellini, andrii_anisov, Konrad Rzeszutek Wilk,
	George Dunlap, Tim Deegan, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, Wei Liu

On 09/11/2018 10:18 AM, Jan Beulich wrote:
>>>> On 11.09.18 at 11:15, <andrew.cooper3@citrix.com> wrote:
>> On 11/09/18 10:10, Jan Beulich wrote:
>>>>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
>>>> On 11.09.18 11:27, Jan Beulich wrote:
>>>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>>>> digit decimal number without any separators is any easier than the
>>>>> current format.
>>>> It's quite subjective. IMHO timestamps measured in ns easier to 
>>>> understand in decimals rather than in separated 32-bit hex-es. No matter 
>>>> how many digital number they have.
>>> It is subjective, yes, but in such a case you even more so need to
>>> demonstrate a change is an overall improvement.
>>>
>>>> Even post processing of perfc output is easier in case of decimal 
>>>> timestamps. You should not parse hexes and odd separators to calculate 
>>>> the time elapsed between two samples.
>>> Post processing usually uses scrips for parsing - I don't think it's
>>> overly complicated to have a script convert the number into
>>> basically any format you want.
>>
>> Right, but this particular mis-pattern wants fixing.  It is simply
>> confusing to have a single number formatted with a colon inbetween.
> 
> What other separator do you suggest? I consider it quite helpful
> to have one when there are more than about 10 digits. (I'd also
> be fine, btw, to have the time printed in decimal, but then
> perhaps with ms granularity and 6 digits for the fractional part.)

* We clearly already have a standard way of printing stime
* The places where a colon is used are non-standard, and thus wrong
* His patch makes an improvement by fixing the non-standard timestamp
printing.

If you're not happy with the current PRI_stime, then it should be on you
to propose and code up a change, not Andrii (unless he feels
particularly motivated to do so).

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:20           ` Andrew Cooper
@ 2018-09-11  9:28             ` Jan Beulich
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  9:28 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Stefano Stabellini, andrii_anisov, Konrad Rzeszutek Wilk,
	George Dunlap, Tim Deegan, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, Wei Liu

>>> On 11.09.18 at 11:20, <andrew.cooper3@citrix.com> wrote:
> On 11/09/18 10:18, Jan Beulich wrote:
>>>>> On 11.09.18 at 11:15, <andrew.cooper3@citrix.com> wrote:
>>> On 11/09/18 10:10, Jan Beulich wrote:
>>>>>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
>>>>> On 11.09.18 11:27, Jan Beulich wrote:
>>>>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>>>>> digit decimal number without any separators is any easier than the
>>>>>> current format.
>>>>> It's quite subjective. IMHO timestamps measured in ns easier to 
>>>>> understand in decimals rather than in separated 32-bit hex-es. No matter 
>>>>> how many digital number they have.
>>>> It is subjective, yes, but in such a case you even more so need to
>>>> demonstrate a change is an overall improvement.
>>>>
>>>>> Even post processing of perfc output is easier in case of decimal 
>>>>> timestamps. You should not parse hexes and odd separators to calculate 
>>>>> the time elapsed between two samples.
>>>> Post processing usually uses scrips for parsing - I don't think it's
>>>> overly complicated to have a script convert the number into
>>>> basically any format you want.
>>> Right, but this particular mis-pattern wants fixing.  It is simply
>>> confusing to have a single number formatted with a colon inbetween.
>> What other separator do you suggest? I consider it quite helpful
>> to have one when there are more than about 10 digits. (I'd also
>> be fine, btw, to have the time printed in decimal, but then
>> perhaps with ms granularity and 6 digits for the fractional part.)
> 
> It is currently more common across the base to use the format proposed
> by Andrii.
> 
> For consistently alone, that format should be followed, or *everything*
> should be changed.

_More_ consistency would be desirable, but I don't think there's a
strict need to get there in one step. A patch shouldn't result in less
consistency, I fully agree. If there are other hard to parse (by a
human) time stamps which get logged, I'm all for changing them,
but right here the focus is just on the headlines which some of the
key handlers log (which, as indicated before, I'd indeed like to see
changed all in one go).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:21           ` Andrii Anisov
@ 2018-09-11  9:30             ` Jan Beulich
  2018-09-11  9:49               ` Andrii Anisov
  0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  9:30 UTC (permalink / raw)
  To: andrii_anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel

>>> On 11.09.18 at 11:21, <andrii_anisov@epam.com> wrote:

> On 11.09.18 12:18, Jan Beulich wrote:
>> What other separator do you suggest? I consider it quite helpful
>> to have one when there are more than about 10 digits. (I'd also
>> be fine, btw, to have the time printed in decimal, but then
>> perhaps with ms granularity and 6 digits for the fractional part.)
> Should we think about replacing PRI_stime?

Why not, but that would require adding another %p suffix or
introducing a construct splitting s_time_t values into two pieces
(to be used to hide the two printk() arguments required).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:24     ` George Dunlap
@ 2018-09-11  9:33       ` Jan Beulich
  2018-09-11  9:50         ` Andrii Anisov
  2018-09-11  9:51         ` George Dunlap
  0 siblings, 2 replies; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  9:33 UTC (permalink / raw)
  To: george.dunlap
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, andrii_anisov

>>> On 11.09.18 at 11:24, <george.dunlap@citrix.com> wrote:
> On 09/11/2018 09:50 AM, Andrii Anisov wrote:
>> Hello Jan,
>> 
>> 
>> On 11.09.18 11:27, Jan Beulich wrote:
>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>> digit decimal number without any separators is any easier than the
>>> current format.
>> It's quite subjective. IMHO timestamps measured in ns easier to
>> understand in decimals rather than in separated 32-bit hex-es. No matter
>> how many digital number they have.
>> Even post processing of perfc output is easier in case of decimal
>> timestamps. You should not parse hexes and odd separators to calculate
>> the time elapsed between two samples.
> 
> I agree with this.  Printing in hexadecimal is useful if you need to see
> the bitwise structure of the content; that's of zero use when it comes
> to timestamps.  On the contrary, you almost always want to be able to do
> a subtraction in your head and get at least an order-of-magnitude
> difference between the times, something which is far too difficult in hex.

I disagree - it's a matter of who looks at those numbers. Personally
I'm quite fine doing order-of-magnitude hex math.

> The fact that we even have PRI_stime is evidence that the developer
> community as a whole agrees.
> 
>>>   And then if you/we were to change this, then please
>>> uniformly everywhere - there's even a second instance right in this
>>> same file you change, not to speak of similar ones elsewhere
> 
> Presumably the fact that we have PRI_stime defined means that *most*
> places already use this format, and the places with `:` in the middle
> are outliers.  It's perfectly acceptable to change them one by one;
> changing them all should only be a suggestion.

Well, what should I say - I disagree. I would agree if it would be a
whole lot of work, but there's not that many key handlers to begin
with, and far not all of them print a time stamp in their headlines.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:28           ` George Dunlap
@ 2018-09-11  9:37             ` Jan Beulich
  0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2018-09-11  9:37 UTC (permalink / raw)
  To: george.dunlap
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, andrii_anisov

>>> On 11.09.18 at 11:28, <george.dunlap@citrix.com> wrote:
> On 09/11/2018 10:18 AM, Jan Beulich wrote:
>>>>> On 11.09.18 at 11:15, <andrew.cooper3@citrix.com> wrote:
>>> On 11/09/18 10:10, Jan Beulich wrote:
>>>>>>> On 11.09.18 at 10:50, <andrii_anisov@epam.com> wrote:
>>>>> On 11.09.18 11:27, Jan Beulich wrote:
>>>>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>>>>> digit decimal number without any separators is any easier than the
>>>>>> current format.
>>>>> It's quite subjective. IMHO timestamps measured in ns easier to 
>>>>> understand in decimals rather than in separated 32-bit hex-es. No matter 
>>>>> how many digital number they have.
>>>> It is subjective, yes, but in such a case you even more so need to
>>>> demonstrate a change is an overall improvement.
>>>>
>>>>> Even post processing of perfc output is easier in case of decimal 
>>>>> timestamps. You should not parse hexes and odd separators to calculate 
>>>>> the time elapsed between two samples.
>>>> Post processing usually uses scrips for parsing - I don't think it's
>>>> overly complicated to have a script convert the number into
>>>> basically any format you want.
>>>
>>> Right, but this particular mis-pattern wants fixing.  It is simply
>>> confusing to have a single number formatted with a colon inbetween.
>> 
>> What other separator do you suggest? I consider it quite helpful
>> to have one when there are more than about 10 digits. (I'd also
>> be fine, btw, to have the time printed in decimal, but then
>> perhaps with ms granularity and 6 digits for the fractional part.)
> 
> * We clearly already have a standard way of printing stime
> * The places where a colon is used are non-standard, and thus wrong
> * His patch makes an improvement by fixing the non-standard timestamp
> printing.
> 
> If you're not happy with the current PRI_stime, then it should be on you
> to propose and code up a change, not Andrii (unless he feels
> particularly motivated to do so).

Excuse me, but no. I'm sure there are places where PRI_stime is
fine to use as is, or there are people thinking so. But to me
personally the proposed change is the opposite of an improvement,
and hence I think I'm perfectly fine to object without going and
creating a counter proposal patch myself. If I'm the only one, I
can still be talked into accepting the change, or outvoted.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:30             ` Jan Beulich
@ 2018-09-11  9:49               ` Andrii Anisov
  2018-09-11 14:04                 ` Andrii Anisov
  0 siblings, 1 reply; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11  9:49 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel


On 11.09.18 12:30, Jan Beulich wrote:
>> Should we think about replacing PRI_stime?
> Why not, but that would require adding another %p suffix or
> introducing a construct splitting s_time_t values into two pieces
> (to be used to hide the two printk() arguments required).
I don't think that splitting s_time_i is reasonable.
Introducing %pt for ns granularity and %pT for ms could be an option.

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:33       ` Jan Beulich
@ 2018-09-11  9:50         ` Andrii Anisov
  2018-09-11  9:51         ` George Dunlap
  1 sibling, 0 replies; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11  9:50 UTC (permalink / raw)
  To: Jan Beulich, george.dunlap
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel


On 11.09.18 12:33, Jan Beulich wrote:
> Well, what should I say - I disagree. I would agree if it would be a
> whole lot of work, but there's not that many key handlers to begin
> with, and far not all of them print a time stamp in their headlines.
I'm quite ok with that.

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:33       ` Jan Beulich
  2018-09-11  9:50         ` Andrii Anisov
@ 2018-09-11  9:51         ` George Dunlap
  1 sibling, 0 replies; 21+ messages in thread
From: George Dunlap @ 2018-09-11  9:51 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel, andrii_anisov

On 09/11/2018 10:33 AM, Jan Beulich wrote:
>>>> On 11.09.18 at 11:24, <george.dunlap@citrix.com> wrote:
>> On 09/11/2018 09:50 AM, Andrii Anisov wrote:
>>> Hello Jan,
>>>
>>>
>>> On 11.09.18 11:27, Jan Beulich wrote:
>>>> NAK, for two reasons: I'm not of the opinion that reading a 15 or more
>>>> digit decimal number without any separators is any easier than the
>>>> current format.
>>> It's quite subjective. IMHO timestamps measured in ns easier to
>>> understand in decimals rather than in separated 32-bit hex-es. No matter
>>> how many digital number they have.
>>> Even post processing of perfc output is easier in case of decimal
>>> timestamps. You should not parse hexes and odd separators to calculate
>>> the time elapsed between two samples.
>>
>> I agree with this.  Printing in hexadecimal is useful if you need to see
>> the bitwise structure of the content; that's of zero use when it comes
>> to timestamps.  On the contrary, you almost always want to be able to do
>> a subtraction in your head and get at least an order-of-magnitude
>> difference between the times, something which is far too difficult in hex.
> 
> I disagree - it's a matter of who looks at those numbers. Personally
> I'm quite fine doing order-of-magnitude hex math.

So quick, about how long is 0x10:00000000 ns?

Maybe you know how long that is, but I would posit that if so, you are
among a handful of people worldwide who have happened to train yourself
in that knowledge.  The millions of Xen developers and system
administrators worldwide who don't have that sense, and (like me) have
absolutely no interest in gaining that knowledge.  I have much better
things to do with my time.

Changing it to seconds (i.e., using the decimal point as a "separator")
would make sense, and I'd be in favor of a patch which did something to
that effect.  But PRI_stime is still an improvement (for the vast
majority of our users and developers); we shouldn't let the best become
the enemy of the good.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11  9:49               ` Andrii Anisov
@ 2018-09-11 14:04                 ` Andrii Anisov
  2018-09-11 14:27                   ` George Dunlap
  2018-09-11 14:33                   ` Jan Beulich
  0 siblings, 2 replies; 21+ messages in thread
From: Andrii Anisov @ 2018-09-11 14:04 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper, George Dunlap
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, Tim Deegan,
	Ian Jackson, andrii.anisov, Julien Grall, xen-devel

So what would be the conclusion?

I'm going to change timestamps format to decimal for all keyhandlers. 
But what about the format itself? Does existing plain ns PRI_stime have 
a chance to be acked?

Or should I think of an extended format (seconds with fractions, ms with 
fractions)?

On 11.09.18 12:49, Andrii Anisov wrote:
>
> On 11.09.18 12:30, Jan Beulich wrote:
>>> Should we think about replacing PRI_stime?
>> Why not, but that would require adding another %p suffix or
>> introducing a construct splitting s_time_t values into two pieces
>> (to be used to hide the two printk() arguments required).
> I don't think that splitting s_time_i is reasonable.
> Introducing %pt for ns granularity and %pT for ms could be an option.
>

-- 

*Andrii Anisov*



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11 14:04                 ` Andrii Anisov
@ 2018-09-11 14:27                   ` George Dunlap
  2018-09-11 14:33                   ` Jan Beulich
  1 sibling, 0 replies; 21+ messages in thread
From: George Dunlap @ 2018-09-11 14:27 UTC (permalink / raw)
  To: Andrii Anisov, Jan Beulich, Andrew Cooper, George Dunlap
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, Tim Deegan,
	Ian Jackson, andrii.anisov, Julien Grall, xen-devel

On 09/11/2018 03:04 PM, Andrii Anisov wrote:
> So what would be the conclusion?
> 
> I'm going to change timestamps format to decimal for all keyhandlers.
> But what about the format itself? Does existing plain ns PRI_stime have
> a chance to be acked?
> 
> Or should I think of an extended format (seconds with fractions, ms with
> fractions)?

I am happy with the current PRI_stime.  If you sent such a patch I would
Ack it and argue for it to be accepted.  (Whether I would prevail, of
course, is a different question.)

I think it would be great to have an extended format for time that
printed <seconds>.<nanoseconds>, and would be very happy if you decided
you wanted to do that.  But I don't think you should be required to do it.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] perfc: Print a system time in a convenient format
  2018-09-11 14:04                 ` Andrii Anisov
  2018-09-11 14:27                   ` George Dunlap
@ 2018-09-11 14:33                   ` Jan Beulich
  1 sibling, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2018-09-11 14:33 UTC (permalink / raw)
  To: andrii_anisov
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, andrii.anisov,
	Julien Grall, xen-devel

>>> On 11.09.18 at 16:04, <andrii_anisov@epam.com> wrote:
> So what would be the conclusion?
> 
> I'm going to change timestamps format to decimal for all keyhandlers. 
> But what about the format itself? Does existing plain ns PRI_stime have 
> a chance to be acked?

Well, if everyone else thinks this is a good idea, then I'm sure someone
will ack it (and I certainly wouldn't veto it in such a case).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-09-11 14:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-11  6:53 [PATCH] perfc: Print a system time in a convenient format Andrii Anisov
2018-09-11  8:27 ` Jan Beulich
2018-09-11  8:50   ` Andrii Anisov
2018-09-11  9:10     ` Jan Beulich
2018-09-11  9:15       ` Andrew Cooper
2018-09-11  9:18         ` Jan Beulich
2018-09-11  9:20           ` Andrew Cooper
2018-09-11  9:28             ` Jan Beulich
2018-09-11  9:21           ` Andrii Anisov
2018-09-11  9:30             ` Jan Beulich
2018-09-11  9:49               ` Andrii Anisov
2018-09-11 14:04                 ` Andrii Anisov
2018-09-11 14:27                   ` George Dunlap
2018-09-11 14:33                   ` Jan Beulich
2018-09-11  9:28           ` George Dunlap
2018-09-11  9:37             ` Jan Beulich
2018-09-11  9:19       ` Andrii Anisov
2018-09-11  9:24     ` George Dunlap
2018-09-11  9:33       ` Jan Beulich
2018-09-11  9:50         ` Andrii Anisov
2018-09-11  9:51         ` George Dunlap

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.