1289318 - 1.79 - 9.04% tabpaint (linux64) regression on push a80fdfc128b0c29dc34dd3fc28868252111a5d52 (Sat Jul 16 2016)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Alison Shiue]

Talos has detected a Firefox performance regression from push a80fdfc128b0c29dc34dd3fc28868252111a5d52. As author of one of the patches included in that push, we need your help to address this regression.

Summary of tests that regressed:

  tabpaint summary linux64 opt - 1.79% worse
  tabpaint summary linux64 pgo - 9.04% worse


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

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 Talos jobs in a pushlog format.

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

For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/Talos/Running

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling

Comment 1

[Alison Shiue]

Here is the zooming better view:
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,7f20e2504c722e15e70fde0c06c926f946d6022c,1,1%5D&series=%5Bmozilla-inbound,f91449d883e157f8cb3ce5d6e6ac3144d7b30689,1,1%5D&zoom=1468621207973.146,1468727078782.256,60,90

This issue might be caused by a80fdfc128b0c29dc34dd3fc28868252111a5d52.
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=469d01eebea4e2055553289ce6542fc093460bbd&tochange=a80fdfc128b0c29dc34dd3fc28868252111a5d52

Hi Till, as you are the patch author, can you take a look at this and determine what is the root cause? Thanks!

Comment 2

[Till Schneidereit [:till]]

I'll take a look at this as soon as I can. As I'm on PTO right now, I don't have a good way to investigate. 

If at all possible, please don't back out bug 911216 over this: it tends to bitrot quickly and landing it was pretty hard. I'll definitely investigate this regression and fix it in-place (with an uplift to aurora).

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

thanks Till, we will follow up when you are back next week.  As a note, this will be on Aurora next week, so lets remember to uplift any fixes to Aurora :)

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

this also shows up on osx:
Summary of tests that regressed:

  tabpaint summary osx-10-10 opt - 6.85% worse

Comment 5

[Alison Shiue]

This regression patch a80fdfc128b0 has been merged into Aurora, so please make sure to uplift any fixes to Aurora, thank you.

https://treeherder.mozilla.org/perf.html#/alerts?id=2185

  tabpaint summary osx-10-10 opt - 5.5% worse

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

it is interesting that the linux64 regression didn't show up initially, looking at the graph, I also see a 8.24% regression on linux64 when we merged to Aurora.

:till, as you are back from vacation, can you please look at this bug and give a timeline of when you can look into this?

Comment 7

[Till Schneidereit [:till]]

(In reply to Joel Maher ( :jmaher ) from comment #6)
> it is interesting that the linux64 regression didn't show up initially,
> looking at the graph, I also see a 8.24% regression on linux64 when we
> merged to Aurora.
> 
> :till, as you are back from vacation, can you please look at this bug and
> give a timeline of when you can look into this?

I did start looking into it, but it's not easy to figure out what's going on here.

While some tabpaint tests have regressed others, such as the win32 ones, are entirely unchanged; osx-10-10 opt e10s has even improved substantially.

Win64 pgo is puzzling: the non-e10s version might've become a bit more noisy, but is largely unchanged. The e10s version, OTOH, improved drastically. The weird thing is that it improved with the first landing of bug 911216 part 30 on the 12th and didn't regress when that was backed out.


I have three different hypotheses on what's going on here, but at least the two most actionable of them have serious problems:

1. The new Promise implementation allocates more memory, causing more memory pressure, more GCs, decreased performance.

This would only seem plausible if there were a lot of promises created during the tabpaint test. I checked, each test run creates about 100 promises, which IMO pretty much rules out this explanation.

2. The new Promise implementation causes Promise reactions (i.e., callbacks supplied to a Promise#then() call) to be invoked later than with the old implementation in some circumstances.

This should result in a slowdown across all platforms and configurations, as it'd be a change in high-level logic, which compilers shouldn't have any influence on. At the very least, it's not compatible with some configurations actually seeing a speedup.

3. The substantially different code paths cause completely different results in the compilers' inlining heuristics, resulting in different performance for some code.

The other two explanations ruled out, this seems to be remaining.

I'm working on some memory usage optimizations which we should land, and probably uplift to Aurora, regardless. They might or might not help, but certainly won't hurt. If they don't help, I'm at a bit of a loss here, I'm afraid.

Comment 8

[Till Schneidereit [:till]]

Boris, maybe you have any other ideas on what's happening here?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Not offhand.  Of your list of three things, the second one seems most plausible to me.  If there are tests that regress reliably and the regression can be reproduced on try, it might be worth adding some logging to see when promises invoke their callbacks in both the spidermonkey promise setup and the DOM promise setup to see whether something jumps out at us...

Comment 10

[Till Schneidereit [:till]]

I have an update to this, though no solution yet. While the data is quite noisy, instrumented builds do show that with the new Promise implementation some promises created during tabpaint are resolved later. Both in terms of Milliseconds and, more importantly, in terms of micro-task checkpoints between Promise creation and resolution. Now to find out *why* that happens ...

Comment 11

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 1: Store contents of spec-defined `capabilities` struct in Promise reaction jobs directly.

No need to keep create an additional object for this.

Review commit: https://reviewboard.mozilla.org/r/71606/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71606/

Comment 12

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 2: Make Promise reaction records their own object type with a constructor and all.

That allows them to be unboxed, saving 32 bytes per instance. It's nicer, too.

Review commit: https://reviewboard.mozilla.org/r/71608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71608/

Comment 13

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 3: Merge Promise fulfillment and rejection reaction lists into a single list.

This saves a slot on Promise instances, an Array allocation, and a rejection record per dependent promise.

While the first doesn't in itself save anything (going from 12 to 11 slots doesn't do anything), the second saves 96 bytes per Promise, and the third 64 bytes per dependent Promise.

Review commit: https://reviewboard.mozilla.org/r/71610/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71610/

Comment 14

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 4: Only allocate the Promise reactions array once the first reaction record is added.

Saves 96 bytes on reaction-less promises. It also saves 32 bytes on promises that have up to two reactions: empty arrays are initialized with an allocated length of 8, whereas providing an initial element initializes to 2.

Review commit: https://reviewboard.mozilla.org/r/71612/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71612/

Comment 15

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 5: Port most Promise functions directly involved in Promise resolution from JS to C++.

Importantly, CreateResolvingFunctions, ResolvePromise, and TriggerPromiseReactions have been ported.

This reduces memory usage because before, the `resolve` and `reject` functions stored on a pending Promise kept track of each other and the Promise they belong to using a closure. Now, that state is stored in the functions' extended slots - which they have anyway.

It should also improve performance, as fewer switches between JS and C++ code occur during processing of Promise reaction job lists.

Review commit: https://reviewboard.mozilla.org/r/71614/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71614/

Comment 16

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 6: Don't store a reference to the reject function on Promise instances themselves.

The reject function is reachable via the resolve function, so we can save a slot.

Review commit: https://reviewboard.mozilla.org/r/71616/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71616/

Comment 17

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 7: Store the Promise reactions list in the same slot as the result.

A Promise can only have either a list of reactions, if it's pending, or a result/reason, if it's resolved.

This gets us down to 3 non-debug-info slots. If we now move the debug info into its own object and only allocate that when the debugger is open, Promise instances only need 4 slots.

Review commit: https://reviewboard.mozilla.org/r/71618/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71618/

Comment 18

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 8: Combine Promise state and rejection handling info into a single flags field.

The rejection handling state will be required even without devtools being open once projection rejection tracking events[1] are implemented, so it should always be tracked on the Promise itself. The other debugging state will be moved into a debug-only object referenced via a slot on Promise.

Review commit: https://reviewboard.mozilla.org/r/71620/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71620/

Comment 19

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 9: Port Promise.resolve and Promise.reject to C++ and optimize various common cases.

This adds a lot of C++ code, but it allows us to optimize cases that would be annoying to optimize in JS.

Review commit: https://reviewboard.mozilla.org/r/71622/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71622/

Comment 20

[Till Schneidereit [:till]]

These 9 patches seem to fix the regression: https://treeherder.mozilla.org/perf.html#/graphs?timerange=2592000&series=%5Btry,ceb25e8fd81f15ed82526f7e28bcfd1f5f6d49ea,1,1%5D&series=%5Btry,cda01fa619e132de1147856a0b8ffd0e0891a4de,0,1%5D&series=%5Btry,4177c453ce909c8414bebbdc390ed01f0f802b16,0,1%5D&series=%5Btry,7c4437c63249bbe78da4c12bbd480b4a1484f229,0,1%5D&series=%5Btry,131b50f3b7eaf046ad65669d939e1cf73089ab27,0,1%5D&series=%5Bmozilla-central,7f20e2504c722e15e70fde0c06c926f946d6022c,0,1%5D&series=%5Bmozilla-central,ceb25e8fd81f15ed82526f7e28bcfd1f5f6d49ea,0,1%5D&series=%5Bmozilla-central,cda01fa619e132de1147856a0b8ffd0e0891a4de,0,1%5D&series=%5Bmozilla-central,4177c453ce909c8414bebbdc390ed01f0f802b16,0,1%5D&highlightedRevisions=742a6cdb6c7a&highlightedRevisions=cb878db85b1b

Other platforms are harder to judge, and the optimizations make sense in any case, so I'd like to get this landed and then see what Talos thinks.

Comment 21

[Till Schneidereit [:till]]

Comment on attachment 8781120 [details]
Bug 1289318 - Part 9: Port Promise.resolve and Promise.reject to C++ and optimize various common cases.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/71622/diff/1-2/

Comment 22

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 1: Store contents of spec-defined `capabilities` struct in Promise reaction jobs directly.

No need to keep create an additional object for this.

Review commit: https://reviewboard.mozilla.org/r/71932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71932/

Comment 23

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 2: Make Promise reaction records their own object type with a constructor and all.

That allows them to be unboxed, saving 32 bytes per instance. It's nicer, too.

Review commit: https://reviewboard.mozilla.org/r/71934/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71934/

Comment 24

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 3: Merge Promise fulfillment and rejection reaction lists into a single list.

This saves a slot on Promise instances, an Array allocation, and a rejection record per dependent promise.

While the first doesn't in itself save anything (going from 12 to 11 slots doesn't do anything), the second saves 96 bytes per Promise, and the third 64 bytes per dependent Promise.

Review commit: https://reviewboard.mozilla.org/r/71936/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71936/

Comment 25

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 4: Only allocate the Promise reactions array once the first reaction record is added.

Saves 96 bytes on reaction-less promises. It also saves 32 bytes on promises that have up to two reactions: empty arrays are initialized with an allocated length of 8, whereas providing an initial element initializes to 2.

Review commit: https://reviewboard.mozilla.org/r/71938/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71938/

Comment 26

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 5: Port most Promise functions directly involved in Promise resolution from JS to C++.

Importantly, CreateResolvingFunctions, ResolvePromise, and TriggerPromiseReactions have been ported.

This reduces memory usage because before, the `resolve` and `reject` functions stored on a pending Promise kept track of each other and the Promise they belong to using a closure. Now, that state is stored in the functions' extended slots - which they have anyway.

It should also improve performance, as fewer switches between JS and C++ code occur during processing of Promise reaction job lists.

Review commit: https://reviewboard.mozilla.org/r/71940/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71940/

Comment 27

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 6: Don't store a reference to the reject function on Promise instances themselves.

The reject function is reachable via the resolve function, so we can save a slot.

Review commit: https://reviewboard.mozilla.org/r/71942/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71942/

Comment 28

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 7: Store the Promise reactions list in the same slot as the result.

A Promise can only have either a list of reactions, if it's pending, or a result/reason, if it's resolved.

This gets us down to 3 non-debug-info slots. If we now move the debug info into its own object and only allocate that when the debugger is open, Promise instances only need 4 slots.

Review commit: https://reviewboard.mozilla.org/r/71944/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71944/

Comment 29

[Till Schneidereit [:till]]

Attachment: Bug 1289318 - Part 8: Combine Promise state and rejection handling info into a single flags field.

The rejection handling state will be required even without devtools being open once projection rejection tracking events[1] are implemented, so it should always be tracked on the Promise itself. The other debugging state will be moved into a debug-only object referenced via a slot on Promise.

Review commit: https://reviewboard.mozilla.org/r/71946/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71946/

Comment 30

[Till Schneidereit [:till]]

(Sorry for the churn, I hadn't realized that I need to re-push all mozreview requests if I want to update just one of them.)

Comment 31

[Eric Faust [:efaust]]

Comment on attachment 8781525 [details]
Bug 1289318 - Part 1: Store contents of spec-defined `capabilities` struct in Promise reaction jobs directly.

https://reviewboard.mozilla.org/r/71932/#review71060

::: js/src/builtin/Promise.js:890
(Diff revision 1)
>  
>      let incumbentGlobal = _GetObjectFromIncumbentGlobal();
>      // Step 5.
>      let fulfillReaction = {
>          __proto__: PromiseReactionRecordProto,
> -        capabilities: resultCapability,
> +        resolve: resultCapability.resolve,

This means that we have to do some plumbing here when we add new things to reactions objects, instead of just passing their capabilities through. Is it worth documenting these places in a "// NB:" style comment?

Comment 32

[Eric Faust [:efaust]]

Comment on attachment 8781526 [details]
Bug 1289318 - Part 2: Make Promise reaction records their own object type with a constructor and all.

https://reviewboard.mozilla.org/r/71934/#review71066

Much cleaner.

Comment 33

[Eric Faust [:efaust]]

Comment on attachment 8781527 [details]
Bug 1289318 - Part 3: Merge Promise fulfillment and rejection reaction lists into a single list.

https://reviewboard.mozilla.org/r/71936/#review71070

A nice complexity reduction.

Comment 34

[Eric Faust [:efaust]]

Comment on attachment 8781528 [details]
Bug 1289318 - Part 4: Only allocate the Promise reactions array once the first reaction record is added.

https://reviewboard.mozilla.org/r/71938/#review71088

It would be interesting to consider having a helper to do the set and assert that the state is always pending, but I think it's vacuously true for this.

Comment 35

[Eric Faust [:efaust]]

Comment on attachment 8781529 [details]
Bug 1289318 - Part 5: Port most Promise functions directly involved in Promise resolution from JS to C++.

https://reviewboard.mozilla.org/r/71940/#review71096

We talked about other possible performance investigations here, with moving some of these things from properties to reserved slots, and the addition of a new JSClass. I wonder if it's faster. Do you think it's worth looking into in the future? If so, please file a followup.

::: js/src/builtin/Promise.cpp:110
(Diff revision 1)
> +        if (!GetProperty(cx, reaction, reaction, handlerName, &handler))
> +            return false;
> +        MOZ_ASSERT((handler.isNumber() &&
> +                    (handler.toNumber() == PROMISE_HANDLER_IDENTITY ||
> +                     handler.toNumber() == PROMISE_HANDLER_THROWER)) ||
> +                   handler.toObject().isCallable());

For readability, I would *vastly* prefer to expand these asserts.

MOZ_ASSERT_IF(!handler.isObject(), handler.isNumber());
MOZ_ASSERT_IF(handler.isNumber(), handler.toNumber() == PROMISE_HANDLER_IDENTITY ||
                                  handler.toNumber() == PROMISE_HANDLER_THROWER);
MOZ_ASSERT_IF(handler.isObject(), handler.toObject().isCallable());

or even

#ifdef DEBUG
if (handler.isNumber()) {
    MOZ_ASSERT(handler.toNumber() == PROMISE_HANDLER_IDENTITY || handler.toNumber() == PROMISE_HANDLER_THROWER);
} else {
    MOZ_ASSERT(handler.isObject());
    MOZ_ASSERT(handler.toObject().isCallable());
}
#endif

or something. Merging them, even with the parens and indentation, took me a moment too long to parse.

::: js/src/builtin/Promise.cpp:581
(Diff revision 1)
>      //   promise: [the promise this reaction resolves],
>      //   resolve: [the `resolve` callback content code provided],
>      //   reject:  [the `reject` callback content code provided],
> -    //   handler: [the internal handler that fulfills/rejects the promise]
> +    //   fulfillHandler: [the internal handler that fulfills the promise]
> +    //   rejectHandler: [the internal handler that rejects the promise]
> +    //   incumbentGlobal: [an object from the global that was incumbent when

This maybe belong elsewhere in the stack? looks like an easrlier part.

::: js/src/tests/ecma_6/Promise/promise-basics.js:23
(Diff revision 1)
>  
> +new Promise((res, rej)=> {res('result');  rej('rejection'); })
> +  .catch(val=>{throw new Error("mustn't be called");})
> +  .then(val=>results.push('then after resolve+reject with val: ' + val),
> +        val=>{throw new Error("mustn't be called")});
> +  

nit: whitespace on blank line.

Comment 36

[Eric Faust [:efaust]]

Comment on attachment 8781530 [details]
Bug 1289318 - Part 6: Don't store a reference to the reject function on Promise instances themselves.

https://reviewboard.mozilla.org/r/71942/#review71128

Nice reduction.

::: js/src/builtin/Promise.cpp:685
(Diff revision 1)
>  {
>      if (this->getFixedSlot(PROMISE_STATE_SLOT).toInt32() != unsigned(JS::PromiseState::Pending))
>          return true;
>  
>      RootedValue funVal(cx, this->getReservedSlot(PROMISE_RESOLVE_FUNCTION_SLOT));
> +    // TODO: ensure that this holds for xray'd promises. (It probably doesn't)

Wow. That's a scary comment. Did anything ever happen with this? Does it need to be addressed?

Comment 37

[Eric Faust [:efaust]]

Comment on attachment 8781531 [details]
Bug 1289318 - Part 7: Store the Promise reactions list in the same slot as the result.

https://reviewboard.mozilla.org/r/71944/#review71132

I guess there are already asserts in the rest of the places that expect results that state != pending. If not, please add.

Comment 38

[Eric Faust [:efaust]]

Comment on attachment 8781532 [details]
Bug 1289318 - Part 8: Combine Promise state and rejection handling info into a single flags field.

https://reviewboard.mozilla.org/r/71946/#review71136

Works for me.

::: js/src/builtin/Promise.cpp:734
(Diff revision 1)
>          }
>      }
>      promise->setFixedSlot(PROMISE_RESOLUTION_SITE_SLOT, ObjectOrNullValue(stack));
>      promise->setFixedSlot(PROMISE_RESOLUTION_TIME_SLOT, DoubleValue(MillisecondsSinceStartup()));
>  
> -    if (promise->state() == JS::PromiseState::Rejected &&
> +    if (promise->state() == JS::PromiseState::Rejected && promise->markedAsUncaught())

nit: mild preference for some word other than "marked". I visually grepped this as "markAsUncaught" first.

Comment 39

[Eric Faust [:efaust]]

Comment on attachment 8781120 [details]
Bug 1289318 - Part 9: Port Promise.resolve and Promise.reject to C++ and optimize various common cases.

https://reviewboard.mozilla.org/r/71622/#review71176

::: js/src/tests/ecma_6/Promise/promise-basics.js:40
(Diff revision 2)
>  assertEq(results[4], 'then after catch with val: 2');
>  assertEq(results[5], 'then after resolve+reject with val: result');
>  
> +results = [];
> +
> +Promise.resolve('resolution').then(res=>results.push(res), 

nit: whitespace at end of line, and on line 23

Comment 40

[Till Schneidereit [:till]]

Comment on attachment 8781529 [details]
Bug 1289318 - Part 5: Port most Promise functions directly involved in Promise resolution from JS to C++.

https://reviewboard.mozilla.org/r/71940/#review71096

> This maybe belong elsewhere in the stack? looks like an easrlier part.

Not really: I forgot to amend the comment when adding the `incumbentGlobal` field in a patch that has already landed.

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ceb37552a97
Part 1: Store contents of spec-defined `capabilities` struct in Promise reaction jobs directly. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/a58a19199304
Part 2: Make Promise reaction records their own object type with a constructor and all. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/436d0a7bcb43
Part 3: Merge Promise fulfillment and rejection reaction lists into a single list. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b97a1aa491a
Part 4: Only allocate the Promise reactions array once the first reaction record is added. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ab4d5b055e4
Part 5: Port most Promise functions directly involved in Promise resolution from JS to C++. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/b46d3d4e8c84
Part 6: Don't store a reference to the reject function on Promise instances themselves. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a3c34e6074a
Part 7: Store the Promise reactions list in the same slot as the result. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/e819902b13a9
Part 8: Combine Promise state and rejection handling info into a single flags field. r=efaust
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd28f500db07
Part 9: Port Promise.resolve and Promise.reject to C++ and optimize various common cases. r=efaust

Comment 42

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4ceb37552a97
https://hg.mozilla.org/mozilla-central/rev/a58a19199304
https://hg.mozilla.org/mozilla-central/rev/436d0a7bcb43
https://hg.mozilla.org/mozilla-central/rev/3b97a1aa491a
https://hg.mozilla.org/mozilla-central/rev/0ab4d5b055e4
https://hg.mozilla.org/mozilla-central/rev/b46d3d4e8c84
https://hg.mozilla.org/mozilla-central/rev/4a3c34e6074a
https://hg.mozilla.org/mozilla-central/rev/e819902b13a9
https://hg.mozilla.org/mozilla-central/rev/cd28f500db07

Comment 43

[Joel Maher ( :jmaher ) (UTC -8)]

I have verified this on the graphs, are we interested in uplifting this to Aurora where the regression was introduced?

Comment 44

[Ryan VanderMeulen [:RyanVM]]

(In reply to Joel Maher ( :jmaher) from comment #43)
> I have verified this on the graphs, are we interested in uplifting this to
> Aurora where the regression was introduced?

Comment 45

[Sylvestre Ledru [:Sylvestre]]

Naveed, what do you think? Thanks

Comment 46

[Jan de Mooij [:jandem]]

These patches are pretty big to uplift to beta IMO.

Comment 47

[Naveed Ihsanullah [:naveed]]

Yes Till and I agree this is too big too uplift.