Open Bug 1741786 Opened 3 years ago Updated 1 year ago

Performance regression detected between m-c build and try build with the same revision

Categories

(Tree Management :: Perfherder, defect, P3)

Tracking

(Not tracked)

UNCONFIRMED

People

(Reporter: arai, Unassigned)

References

Details

(Whiteboard: [fxp])

https://treeherder.mozilla.org/perfherder/compare?originalProject=mozilla-central&originalRevision=b16763f1da6bcf31f964744c765c4af480ae551c&newProject=try&newRevision=df12762eef9a5e7b6af281e8a94cbdc03313d066&framework=13&page=1&showOnlyComparable=1&showOnlyImportant=1

The comparison above is Browsertime tp6 microsoft, on Linux 18.04 x64 shippable build,
for revision b16763f1da6bcf31f964744c765c4af480ae551c both on m-c and try.
(try one has try commit, but its parent is b16763f1da6bcf31f964744c765c4af480ae551c)

The result shows 2-5% regression, with 21 runs, and the confidence is med to high.

It suggests that there's some difference in either:

  • the generated binary (maybe somewhere around PGO?)
    • at least I see several binary files are different between those 2 pushes. haven't looked into the details how they're different tho
  • the test hardware

similar problem is reported before in bug 1515681, but that was about comparing try against recent m-c pushes,
and it's fixed by adding the following warning on perfherder

It is not recommended to compare a try build against a mozilla-central build, unless it is based on latest mozilla-central.

This bug is about comparing the same revision built on try push and m-c push, so the above warning doesn't apply.

If there's actually some difference between the try push and m-c push, we should disallow comparing them, and show a message that suggests pushing the base revision also to try,
or at least reword the warning message on perfherder comparison page, and also show the warning on the comparison result page as well.

Beatrice/Andra, could one of you look into this?

Flags: needinfo?(beatrice.acasandrei)
Flags: needinfo?(aesanu)

My understanding is that both cases mentioned by you (comparing try against recent m-c pushes and comparing the same revision built on try push and m-c push) are covered by the default message shown to not compare:

  • a base try build against a new mozilla-central build
  • a base mozilla-central build against a new try build

This message will only show when someone attempts to compare try results with m-c results.

I agree with your suggestions to show a message about pushing the base revision also to try and also show the warning on the comparison result page as well. If you would like to reword the warning message in a certain one please let me know how.

Flags: needinfo?(beatrice.acasandrei)
Flags: needinfo?(aesanu)

Do you mean this message? or maybe I overlooked some other message?

It is not recommended to compare a try build against a mozilla-central build, unless it is based on latest mozilla-central.

for me, it sounds like "do not compare try vs m-c if base revisions are far different", especially by the "unless it is based on latest mozilla-central" part that adds an exception to the first sentence.
and, my understanding from it is "it's fine to compare try vs m-c if the try push is based/rebased onto latest m-c",
that's the situation where I hit the issue.

So, to my understanding, "comparing the same revision built on try push and m-c push" case isn't covered by the message,
and also it doesn't imply underlying performance characteristics difference between try vs m-c pushes, that's the actual issue the developer should be aware of, and the reasoning why "not recommended".

I'd like to reword it to something like:

"It is not recommended to compare a try build against a mozilla-central build. the performance characteristics can be different between them. Please consider pushing the mozilla-central revision also to try server and comparing between try builds."

Also, if the performance characteristics difference and the underlying binary difference are real, is there any valid scenario for comparing a try build vs a m-c build?
if there isn't any, disallowing the comparison will prevent similar trouble in the future.

Also, it would be better figuring out the detail where the performance difference comes from, and note it somewhere,
and maybe link to it from the warning message.
There are many more projects that can be chosen for base/new in perfherder, and I don't think only "try vs m-c" combination is affected by the performance difference.

Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.