1619502 - 1.12 - 1.22% Base Content JS (windows7-32, windows7-32-shippable) regression on push d93e9fef86dd098bfce6b2e7b16da0366298a4b0 (Fri February 28 2020)

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P3)

Description

[Alexandru Ionescu (needinfo me) [:alexandrui]]

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=d93e9fef86dd098bfce6b2e7b16da0366298a4b0

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

1% Base Content JS windows7-32 opt 2,922,497.33 -> 2,958,189.33
1% Base Content JS windows7-32-shippable opt 2,922,498.67 -> 2,958,190.67
1% Base Content JS windows7-32-shippable opt 2,925,484.67 -> 2,958,314.67

You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=25252

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/AWSY/Tests

Comment 1

[André Bargull [:anba]]

I've performed the following two tests to determine why the extra memory is allocated.

1. In CloneFunctionReuseScript, I've commented out these lines to test if the extra memory is needed because the cloned function is no longer nursery allocated or if it is related to creating a new ObjectGroup. The comparison shows that commenting out these lines reduces "Base Content JS" by 1.25%. That means the extra memory is not needed for the tenured allocation, but rather it is related to creating a new ObjectGroup.
2. Additionally I've tested what happens when we modify the original's ObjectGroup in-place in order to avoid creating a whole new ObjectGroup. The comparison shows no difference in "Base Content JS". That can mean one of two things:
   1. The added optimisation doesn't apply for "Base Content JS", which would mean some Firefox code is creating many derived classes with varying parent classes. This seems unlikely and I also don't see any indication of this when skimming through the source code.
   2. The extra memory is needed because we're now running optimisations code paths, which were previously not applied, because the required optimisation information wasn't present.

Right now I have to assume 2.ii applies, in which case the increased memory usage is unavoidable. WarpBuilder should help here, but that project has just been started.

Comment 2

[André Bargull [:anba]]

I've no idea why "status-thunderbird75" was reset by my comment. I also don't seem to be able to revert that change, because "thunderbird75" isn't listed for me under "Tracking Flags"...

Comment 3

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to André Bargull [:anba] from comment #2)

I've no idea why "status-thunderbird75" was reset by my comment. I also don't seem to be able to revert that change, because "thunderbird75" isn't listed for me under "Tracking Flags"...

Bebe ^

Comment 4

[Florin Strugariu [:Bebe]]

My bad... sorry for the status-thunderbird75... it should be firefox 75

Comment 5

[Julien Cristau [:jcristau]]

Sounds like a wontfix for 75.

Comment 6

[Greg Mierzwinski [:sparky]]

Closing as WONTFIX based on comment #1.