[PATCH v2 2/5] perf jevents: Match on highest version of Arm json file available

[John Garry]

>>>
>> What's the end goal of reducing duplication? Is it to reduce the binary
>> size or to reduce human input?

Not to reduce binary size. Well that was never the original intent.

Originally we were seeing many transcribed JSONs, with descriptions 
manually copied from TRMs/ARM ARM or some random implementator 
speadsheets. There was also lots of inconsistencies of event 
descriptions and classifications between implementators and 
implemenations when there are no differences in practice – it is not 
helpful in giving a consistent user experience on arm.

The motivation to reduce duplication was for the usual reasons, like:
- minimize mistakes
- ease of maintenance
- easier to update
- etc

There was no central repository of ARM events for all implementators, 
like intel had in (I think) 01org. So it was good to try to provide a 
method in perf tool to harmonize event support there.

Obviously when implementators have their own repo of events in 
proprietary formats - like arm does - some conversion needs to be done 
(to perf tool format). And using methods in perf tool there to reduce 
duplication becomes questionable - reducing duplication would be the job 
there of the original repo author.

So we could say that arm can have as much duplication as they want in 
their JSONs as we know that they are managing in their own repo, but 
having different rules for different implementators is going to be 
difficult to manage. But allowing duplication in arm JSONs does leave 
door open for inconsistent event descriptions between implementators.

>>
>> For the binary size if most of these strings are the same then they are
>> de-duplicated and the impact on the binary size is very small.
>>
>> If it's to save on human input, then not doing it this way has the
>> potential to be more work in the long run. At the moment we can just run
>> a script and generate the Arm JSONs using what Arm is publishing. We
>> might understand all these subtle de-duplication efforts, but anyone
>> coming in the future is going to have a hard time understanding.
>> Especially if they just want to re-run the script to update some text,
>> then there is a big risk of making a mistake when re-applying the manual
>> changes.
>>
>> I'm not completely opposed to the idea of re-working this expression
>> this time so that it works in both places, but I think we should be sure
>> that we do it for the right reasons.

For sure. For this issue, it's ironically a bit annoying to have such a 
small set of differences - otherwise we could just let it in as is. But 
have a small set of subtle differences in a handful or metrics makes me 
think that we can solve this with literal / formula.

Again, if this turns out to be a continuous issue popping up, then it 
may become an unmanageable rule.

>>
>> As an example for comparison, I tried a build with a single set of
>> events and both sets (from this change) and the difference in size is
>> 1760 bytes. Is it really worth starting to edit formulas just to save
>> that much space?
>>
> I re-ran the test with a clean build and the difference is actually 4888
> bytes. Still quite small but not as suspiciously small as 1760.

But it's still not huge in terms of size.

FWIW, we could prob reduce size more by doing the archstdevent fixup 
during runtime and not in generating pmu-events.c, which would be saving 
more space.

> 
>>>> It's possible in the future that there are also small changes to the
>>>> description text or availability of the events between versions. It
>>>> hasn't happened yet, but it gives us the flexibility to do that too.
>>> Here the (handful of) metrics are subtly different between these
>>> rev/variant, but all the events are the same. It's hard to justify
>>> duplicating almost everything just for that.
>>>
>> If it saves time and reduces the chance of making mistakes then
>> personally I don't see an issue with the duplication.
>>
>>> I am not sure on how to handle this, since we might be bombarded with
>>> support for more CPUs with the same MIDR issue and having quirks for all
>>> becomes unworkable.
>>>
>> I really don't see this being used very often. And it will always be
>> used much less often than the task of adding entirely new CPUs, so it
>> won't cause the number of JSONs to grow faster than that already
>> existing work. To be honest there is a non-zero chance that it never
>> gets used again.
>>
>> Although even if we never use it again we still might want to
>> re-generate the JSONS because of a typo fix or something so that task is
>> just a script run rather than script run, and then re-applying the
>> expression fixes.
>>
>>> Can you just give the idea above on literals/functions in the metricexpr
>>> a go, to see how it looks? Maybe Ian has a good idea on similar solutions.
>>>
>>> Thanks,
>>> John
>> Yes I can give it a go. Also interested to see what Ian thinks as well.

Cheers for that.

John