1571619 - 0.27 - 0.34% Base Content JS (linux64-shippable, linux64-shippable-qr, macosx1014-64-shippable, windows7-32-shippable) regression on push 9973154c79e50bcb7fa5cce165230d8ebce42452 (Fri August 2 2019)

Status: RESOLVED WONTFIX

(DevTools :: Debugger, defect, P2)

Description

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

We have detected an awsy regression from push:

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

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

Regressions:

0.34% Base Content JS windows7-32-shippable opt 3,239,765.33 -> 3,250,885.33
0.27% Base Content JS linux64-shippable opt 4,129,254.67 -> 4,140,272.00
0.27% Base Content JS linux64-shippable-qr opt 4,129,334.67 -> 4,140,298.67
0.27% Base Content JS macosx1014-64-shippable opt 4,130,177.33 -> 4,141,354.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=22255

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

[Jason Laster [:jlast]]

Hi Alexandru, what is the best way to try and reproduce this locally? This is a bit surprising to me given how minor the patch was.

Also, CCing Logan who might have more context

Comment 2

[Logan Smyth [:loganfsmyth]]

The main logical change here adds a new branch in what is probably a relatively common code path, so perhaps that was enough to tweak the metrics?

Comment 3

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

(In reply to Jason Laster [:jlast] from comment #1)

Hi Alexandru, what is the best way to try and reproduce this locally? This is a bit surprising to me given how minor the patch was.

Also, CCing Logan who might have more context

Jason, I'm affraid you know better than me what you did in this package.
Did you got to a conclusion so far?

Comment 4

[Jason Laster [:jlast]]

Redirecting to Jim, who is more familiar with investigating performance regressions in the debugger frontend api.

Comment 5

[Jim Blandy :jimb]

(In reply to Logan Smyth [:loganfsmyth] from comment #2)

The main logical change here adds a new branch in what is probably a relatively common code path, so perhaps that was enough to tweak the metrics?

I don't think it's a speed regression, I think it's a memory consumption regression, based on the explanation of the AWSY "Base Content JS" tests:

Base Content JS

An updated test focused on supporting fission. This measures the base overhead of an empty content process. It tracks resident unique, heap unclassified, JS, and explicit memory metrics as well as storing full memory reports as artifacts. The median value for each metric is used from across all content processes. It has much lower thresholds for alerting and is recorded in Perfherder.

I think what's happening is that the patch caused a significant increase in the number of source notes.

Comment 6

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

(In reply to Jim Blandy :jimb from comment #5)

(In reply to Logan Smyth [:loganfsmyth] from comment #2)

The main logical change here adds a new branch in what is probably a relatively common code path, so perhaps that was enough to tweak the metrics?

I don't think it's a speed regression, I think it's a memory consumption regression, based on the explanation of the AWSY "Base Content JS" tests:

Base Content JS

An updated test focused on supporting fission. This measures the base overhead of an empty content process. It tracks resident unique, heap unclassified, JS, and explicit memory metrics as well as storing full memory reports as artifacts. The median value for each metric is used from across all content processes. It has much lower thresholds for alerting and is recorded in Perfherder.

I think what's happening is that the patch caused a significant increase in the number of source notes.

jlaster, thoughts on this?

Comment 7

[Jason Laster [:jlast]]

I agree with Jim.

We landed a patch two weeks after (bug 1577047) that significantly reduced the memory footprint.

At this point, i think the increased memory usage is necessary given that we want to improve stepping quality while debugging.