Re: [Minios-devel] [UNIKRAFT PATCH 6/8] support/scripts: implement trace buffer parcing

[Costin Lupu <costin.lup@xxxxxxxxx>]

On 5/29/19 6:54 PM, Yuri Volchkov wrote:
> Costin Lupu <costin.lup@xxxxxxxxx> writes:
> 
>> On 5/29/19 4:42 PM, Yuri Volchkov wrote:
>>> Hi,
>>>
>>> Costin Lupu <costin.lup@xxxxxxxxx> writes:
>>>
>>>> Hi Yuri,
>>>>
>>>> In the subject: s/parcing/parsing .
>>> Ack
>>>
>>>>
>>>> Please see my comments inline.
>>>>
>>>> On 5/10/19 9:29 PM, Yuri Volchkov wrote:
>>>>> With this patch trace.py can print the data collected in the
>>>>> tracefile. And two more use cases are covered for convince:
>>>>
>>>> s/for convince/for convenience/
>>> Ack
>>>
>>>>
>>>>> +
>>>>> +class tp_sample:
>>>>
>>>> It can get confusing on why there are 2 classes for tracepoints,
>>>> tracepoint and tp_sample. I think their names should reflect that the
>>>> former are the tracepoint definitions and latter are the tracepoint values.
>>> Hm, I thought that tracepoint sample is a lot more clear that is a
>>> _sample_ of a tracepoint, than "tracepoint value". From merriam-webster
>>> dictionary (https://www.merriam-webster.com/dictionary/sample):
>>>
>>>     1) a representative part or a single item from a larger whole or
>>>        group especially when presented for inspection or shown as
>>>        evidence of quality
>>>     
>>>     2) finite part of a statistical population whose properties are
>>>        studied to gain information about the whole
>>
>> Oh, and I thought that one of the purposes of reviewing was to help the
>> author with suggestions in order to make the code even more clearer, you
>> know, for other people than the author.
> You thought very right. I did not mean to upset you. I really described
> my thought process. I really thought (and actually still do) that
> 'tp_sample' is a better name to a sample of tracepoint then 'value'. If
> you are still not happy with the name, I can offer you 2 other names:
> 'tp_sample_data' 'tp_event_data'. However it makes it a little too
> long. What do you think?

One is for meta-data ('tracepoint') and the other is for the actual data
('tp_sample'). That's what the names should reflect (or some comments in
the code, something to make it obvious), that's why in my opinion
'tracepoint_definition' and 'tracepoint_data' would suit.
'tracepoint_sample' is good too as long as there is a
'tracepoint_definition' counterpart (I don't care if it's 'sample' or
'value'). And I am also looking for consistency in the naming, either
'tracepoint_*' or 'tp_*'.

To recap, these are the simplest options:
- for meta: tracepoint_definition, tracepoint_meta, tp_definition, etc
- for data: tracepoint_data, tracepoint_sample, tp_sample

Costin


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel