Re: [PATCH RFC 0/6] Add metrics for neoverse-n2

[James Clark]

On 19/11/2022 21:46, Ian Rogers wrote:
> On Fri, Nov 18, 2022 at 7:30 PM Jing Zhang <renyu.zj@xxxxxxxxxxxxxxxxx>
> wrote:
>>
>>
>> 在 2022/11/16 下午7:19, James Clark 写道:
>>>
>>>
>>> On 31/10/2022 11:11, Jing Zhang wrote:
>>>> This series add six metricgroups for neoverse-n2, among which, the
>>>> formula of topdown L1 is from the document:
>>>>
> https://documentation-service.arm.com/static/60250c7395978b529036da86?token=
>>>>
>>>> Since neoverse-n2 does not yet support topdown L2, metricgroups such
>>>> as Cache, TLB, Branch, InstructionsMix, and PEutilization are added to
>>>> help further analysis of performance bottlenecks.
>>>>
>>>
>>> Hi Jing,
>>>
>>> Thanks for working on this, these metrics look ok to me in general,
>>> although we're currently working on publishing standardised metrics
>>> across all new cores as part of a new project in Arm. This will include
>>> N2, and our ones are very similar (or almost identical) to yours,
>>> barring slightly different group names, metric names, and differences in
>>> things like outputting topdown metrics as percentages.
>>>
>>> We plan to publish our standard metrics some time in the next 2 months.
>>> Would you consider holding off on merging this change so that we have
>>> consistant group names and units going forward? Otherwise N2 would be
>>> the odd one out. I will send you the metrics when they are ready, and we
>>> will have a script to generate perf jsons from them, so you can review.
>>>
>>> We also have a slightly different forumula for one of the top down
>>> metrics which I think would be slightly more accurate. We don't have
>>> anything for your "PE utilization" metrics, which I can raise
>>> internally. It could always be added to perf on top of the standardised
>>> ones if we don't add it to our standard ones.
>>>
>>> Thanks
>>> James
>>>
>>
>> Hi James,
>>
>> Regarding the arm n2 standard metrics last time, is my understanding
> correct,
>> and does it meet your meaning? If so, may I ask when you will send me the
>> standards you formulate so that I can align with you in time over my
> patchset.
>> Please communicate this matter so that we can understand each other's
> schedule.
>>
>> Thanks,
>> Jing
> 
> Hi,
> 
> In past versions of the perf tool the metrics have been pretty broken. If
> we have something that is good we shouldn't be holding it to a bar of being
> perfect, we can merge what we have and improve over time. In this case what
> Jing has prepared may arrive in time for Linux 6.2 whilst the standard
> metrics may arrive in time for 6.3. I'd suggest merging Jing's work and
> then improving on it with the standard metrics.
>

I'm not completely opposed to this, I was just worried about the churn
because ours will be generated from a script, and that it would end up
looking like a mass replacement of these that would have only recently
been added.

But maybe that's fine like you say.

> In terms of the metrics themselves, could we add ScaleUnit? For example:
> 
> +    {
> +        "MetricExpr": "LD_SPEC / INST_SPEC",
> +        "PublicDescription": "The rate of load instructions speculatively
> executed to overall instructions speclatively executed",
> +        "BriefDescription": "The rate of load instructions speculatively
> executed to overall instructions speclatively executed",
> +        "MetricGroup": "InstructionMix",
> +        "MetricName": "load_spec_rate"
> +    },
> 
> A ScaleUnit of "100%" would likely make things more readable.
> 
> Thanks,
> Ian
>