1301771 - browser hangs on github merge/create pull request

Status: RESOLVED WORKSFORME

(Firefox :: Untriaged, defect)

Description

[heidi.kasemir]

Attachment: Screen Shot 2016-09-09 at 12.34.08 PM.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce:

FF developer edition 50.0a2 (2016-09-09) - and earlier, but this is latest I can reproduce on.

Go to a github repo where you can make a pull request to merge one branch into another.

Open the pull request and start typing the description/title.


Actual results:

Browser hangs. The page is frozen, I can't close or switch tabs. The cursor turns into a spinning rainbow wheel (I'm on Mac OSX v10.11.6), and Activity monitor shows FirefoxDeveloperEdition and FFDE WebContent as frozen processes.

I have to force quit the processes to close and restart the browser.


Expected results:

I should have been able to open a PR, add descriptive information and save.

Comment 1

[heidi.kasemir]

reproduced on nightly 51.0a1 (2016-09-08) (64-bit) - took 3 minutes before the page was responsive again.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

I tried to reproduce it on Linux.
STR I used:

1) Go to github.com
2) Log in
3) Go to an already existing PR
4) Click on "edit" next to the PR title
5) Edit the title

If I understand the reported correctly it should result in a hang, but it works for me.

May be Mac only.

Comment 3

[heidi.kasemir]

Attachment: Notification window with unresponsive script

Latest attempt to re-reproduce on developer edition: browser hung for two minutes, then a notification window opened with this message

Comment 4

[Loic]

Heidi, did you test with e10s disabled?

Comment 5

[heidi.kasemir]

Hi Loic - I just gave that a try and it essentially fixed it! The page seemed to pause for only a half a second or so before allowing me to continue making the PR.

Comment 6

[Loic]

I'm not a user of Github, is there a public repo where I can make a "random" pull request to reproduce the bug?

maybe there is a regression in FF51.

Comment 7

[alex_mayorga]

¡Hola Heidi!

Could you please try the steps detailed at https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Reporting_a_Performance_Problem and provide a profile for this bug?

¡Gracias!
Alex

Comment 8

[heidi.kasemir]

Hey there Alex - I gave that a try - hopefully this works: https://cleopatra.io/#report=09529d1d59878a636b66b885d314d028a8479a25.

I made a teeny public repo for testing purposes: https://github.com/hkasemir/test so Loic - if you want to attempt making a PR - I'm not sure if you can do this without making a github profile, but you can work with this if you like.

I think the length of the stall-out is correlated with the size of the PR as when I attempt to make a PR to merge the branch testing-branch, it only hangs for a few seconds, where if I'm making a significantly larger one it seems to hang for several minutes.

Let me know if this doesn't give the data you need.

I decided to try with a bigger repo and bigger PR: https://cleopatra.io/#report=7d7500c774c9d91d2f9f830c73761bf093b3631c

Comment 9

[heidi.kasemir]

oops! realized I had forgotten to update my nightly - here's another data point from the latest version, with the larger PR. https://cleopatra.io/#report=d5e9e971d6d4706f26119808636af7e87074959c

Comment 10

[Mike Conley (:mconley) (:⚙️)]

According to this profile, a good chunk of this performance problem is caused by LastPass. heidi.kasemir, if you temporarily disable LastPass, does the issue go away?

Comment 11

[heidi.kasemir]

Interesting! That seems to have worked really quite well -

Here's the profile when lastpass was enabled: https://cleopatra.io/#report=473e9e2e1868946433b5f871a7eb4e60e9b786af

and with lastpass disabled:
https://cleopatra.io/#report=7e36be5717d5964ffecaa2e18c04da52adae92f1

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

There is a new version of LastPass extension (4.1.*) which fixes this bug for the reported. Unfortunately, AMO is offering the old one (3.x) which doesn't work with e10s.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

(In reply to heidi.kasemir from comment #11)
> Interesting! That seems to have worked really quite well -
> 
> Here's the profile when lastpass was enabled:
> https://cleopatra.io/#report=473e9e2e1868946433b5f871a7eb4e60e9b786af
> 
> and with lastpass disabled:
> https://cleopatra.io/#report=7e36be5717d5964ffecaa2e18c04da52adae92f1

Okay, excellent. Disabling LastPass really cleans things up in here. There's still some badness in here though - I've zoomed in:

https://cleopatra.io/#report=89460edc660b20188988d245bbfbe0a617d79e91&jankOnly=true&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A12488,%22end%22%3A14598%7D%5D&selection=0,6842,6843,6844,6845,6412,6846,8,9,10,11,12,13,14,15,16,17,31,32,33,34,35,36,37,105,106,107,108,109,7216,7217,7218,7219,45,46,47,48,690,691,984,9609,986,987,988,1922,1923,1924,1925,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,5186,1481,1482,5185,6247,6248,6250,7614,7615,7616,6250,7617,7618,7619,7620,9869

Reflow #8 (~100ms) appears to be caused by some SVG flex stuff... maybe having to do with text? We also have a Styles recalculation #61 (~200ms), which is pretty rough.

Hey dholbert, anything actionable in here?

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #13)
> still some badness in here though - I've zoomed in: [link]
> 
> Reflow #8 (~100ms) appears to be caused by some SVG flex stuff...

(mconley's talking about the hover-text from Reflow #8 here, which has some stuff about "set_WebkitFlexWrap" inside of "mozilla::dom::SVGElementBinding")

BenWa says this hover-text is actually just reporting the first thing that happens to set a dirty bit that gets flushed as part of this reflow -- so (when a lot of page changes happen at once) it's not actually a good sign of the real culprit.

Drilling down into the samples (after clicking on the "content" timeline and selecting the range around Reflow #8), it looks like it's all deeply-nested calls into nsBlockFrame reflow (with some table reflow mixed in, and text reflow at the deepest levels).

So, nothing jumps out at me as being particularly actionable there -- though it's possible more analysis (and particularly-pathological testcases) might reveal something.

Comment 15

[Daniel Holbert [:dholbert]]

But it sounds like most of the perf issues here were from LastPass & were fixed (though AMO needs an update), which is great news!

Comment 16

[heidi.kasemir]

yep! I'm glad it was that simple to fix! Thanks for helping me get to the bottom of it :D