1368119 - Nightly with ABP WebExtension janks hard for 3-4 seconds, and the memory of 1 contentprocess doubles

Status: RESOLVED WORKSFORME

(WebExtensions :: General, defect, P2)

Description

[Mayank Bansal]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170526030203

Steps to reproduce:

Approximate STR:

1. Use normal profile
2. Start nightly
3. Go to a longish page like https://bugzilla.mozilla.org/show_bug.cgi?id=1362866
4. Scroll on it for 30-40 seconds or so


Actual results:

One of the content process will jank hard for 3-4 seconds. The memory usage of that process doubles during this period (From 350MB to 710MB)

This has started happening very recently. most probably only 1-2 nightlies back
Im on Win10x64

Here is a profile I took : https://perfht.ml/2rYYS2U


Expected results:

not so.

Comment 1

[Mayank Bansal]

tentative ni?.
These are approximate STR, unfortunately.

Comment 2

[Mayank Bansal]

All this happens within 2 minutes of starting the browser. Maybe 1 minute.
The increased memory goes down after it stops janking.
The profile i have attached, definitely captures the start of the jank. I may have prematurely stopped the profiler before the end of jank.

Comment 3

[Mayank Bansal]

hopefully a better profile:
https://perfht.ml/2rpKdk2

Comment 4

[(no longer active)]

Thank you for the great bug report, Mayanak!

May I ask what extensions you have installed?

From the profile, it seems like an extension is trying to write some JSON data to a storage API and the serialization for it is going through cross compartment wrappers in the parent process and that's really slow...  CCing Kris and Andrew in case this hints something to them.

Comment 5

[(no longer active)]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #4)
> Thank you for the great bug report, Mayanak!

Oops, Mayank, my apologies!  I need better glasses.  :-)

Comment 6

[Mayank Bansal]

Attachment: aboutsupport.txt

About:support

Comment 7

[Kris Maglione [:kmag]]

Ugh. Yes, this is largely bug 1356546 and bug 1320186, but if the extension is writing enough data to the storage API to cause something this bad, then something probably needs to change in the extension, too.

Comment 8

[Kris Maglione [:kmag]]

It seems like that's probably coming from Adblock Plus.

Comment 9

[Mayank Bansal]

yes, it was updates yesterday, which corresponds with my experience.

Comment 10

[Mayank Bansal]

Confirmed it is ABP.
 I had no jank when i disabled it. But after enabling it, within the next 20-30 seconds, the jank happened.

Comment 11

[Kris Maglione [:kmag]]

As long as we're willing to break compatibility with Chrome, and fail when the object contains objects like functions, we can just skip the JSON sanitizer step and do a straight structured clone. I've been considering moving the storage backend to IndexedDB, anyway, and allowing storing arbitrary structured clonable data rather than just JSON-compatible data, so that wouldn't be a bad first step.

Comment 12

[Kris Maglione [:kmag]]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #4)
> May I ask what extensions you have installed?

On that note... The old Gecko Profiler stored the extension IDs that were responsible for a given stack frame, and (sort of) surfaced that in the UI, but the new profiler doesn't seem to have that data even in the raw profile JSON. Was that functionality intentionally removed? Are we planning to re-add it?

Comment 13

[Andy McKay]

Leaving open based on comment 12 although it sounds like we could dupe it to one of the bugs in comment 7.

Comment 14

[Markus Stange [:mstange]]

(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #12)
> On that note... The old Gecko Profiler stored the extension IDs that were
> responsible for a given stack frame, and (sort of) surfaced that in the UI,
> but the new profiler doesn't seem to have that data even in the raw profile
> JSON. Was that functionality intentionally removed? Are we planning to
> re-add it?

I'd like us to re-add it. This information came from the old add-on and I never got around to gathering that same information in the new add-on. And with WebExtensions the rules for detecting which URL maps to which add-on have changed slightly, and some of the old rules might not be necessary any more.

Comment 15

[u595893]

IDK if *i* did it right, fresh with only two extensions

but here's mine with ABP & Ghostery

https://perf-html.io/public/25ead3ba7caf0f4d3f4da7c07f0d6223c8b59b67/calltree/?thread=2


even worse if the start updating/downloading filters

https://perf-html.io/public/c67c412c2e12c356e53a7bbd9d5951fc5d6b212b/calltree/?thread=2

Comment 16

[Kris Maglione [:kmag]]

(In reply to Markus Stange [:mstange] from comment #14)
> I'd like us to re-add it. This information came from the old add-on and I
> never got around to gathering that same information in the new add-on. And
> with WebExtensions the rules for detecting which URL maps to which add-on
> have changed slightly, and some of the old rules might not be necessary any
> more.

OK. Thanks. Let me know if there's anything I can do to help with that.

I don't think WebExtensions should change things that much. Either way, we should probably just be using the add-on path service, and check any URL prefix that matches something like `(moz-extension|resource)://[^/]+/|chrome://[^/]+/(content|skin|locale)/` and ignore substitutions from custom protocols or about: handlers.

Comment 17

[(no longer active)]

(In reply to shellye from comment #15)
> IDK if *i* did it right, fresh with only two extensions
> 
> but here's mine with ABP & Ghostery
> 
> https://perf-html.io/public/25ead3ba7caf0f4d3f4da7c07f0d6223c8b59b67/
> calltree/?thread=2
> 
> 
> even worse if the start updating/downloading filters
> 
> https://perf-html.io/public/c67c412c2e12c356e53a7bbd9d5951fc5d6b212b/
> calltree/?thread=2

I think those show symptoms similar to the original issue reported here...

Comment 18

[(no longer active)]

(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #7)
> Ugh. Yes, this is largely bug 1356546 and bug 1320186, but if the extension
> is writing enough data to the storage API to cause something this bad, then
> something probably needs to change in the extension, too.

Since both of these bugs are now fixed, I'm gonna call this fixed.  If you experienced this again, please feel free to reopen!

Comment 19

[Kris Maglione [:kmag]]

Unless someone can verify that this is no longer an issue, I think we'll need bug 1370752 before we can call this fixed. From the profiling I've done in the past, I think removing the redundant sanitization should remove about a third of the overhead, but the initial sanitization that takes place through X-rays is still a more serious issue.

Comment 20

[Mayank Bansal]

With Nightly buildid 20170608030205 , and ABP2.9.1 , the jank has reduced by half. Earlier it was around 4-5 seconds. Now it is around 1.5-2.5 seconds.

Comment 21

[Mayank Bansal]

Actually, not sure about any improvement. Maybe 1 second less?
Here is a profile:  https://perfht.ml/2rELVxb  The interesting parts should be near the end.

Comment 22

[Mayank Bansal]

Cant repro this anymore. I'd say this is fixed.