[e10s] Performance regression, Computing hash on virustotal is slow. Nightly54.0a1 is 5-10 times slower than non e10s

VERIFIED FIXED in Firefox 54

Status

()

Core
DOM
VERIFIED FIXED
3 months ago
7 days ago

People

(Reporter: Alice0775 White, Assigned: baku)

Tracking

({multiprocess, perf, regression})

54 Branch
mozilla55
x86
Windows 10
multiprocess, perf, regression
Points:
---

Firefox Tracking Flags

(firefox51 wontfix, firefox52 wontfix, firefox53 wontfix, firefox54+ verified, firefox55 verified, firefox-esr45 disabled, firefox-esr52 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 months ago
[Tracking Requested - why for this release]:

Steps to Reproduce:
1. Open https://www.virustotal.com/en/
2. Chose one of exe file from local filt system (e.g. full installer of Firefox)
3. Clock on [Scan it!], (no needs actual upload)
   --- observe, Computing hash...

Actual Results:
Computing hash is slow on Nightly54.0a1
Nightly54.0a1 is 5-10 times slower than Aurora53.0a2.

Expected Results:
It should be same as Aurora53.0a2
Do you have a regression range?
(Reporter)

Comment 2

3 months ago
(In reply to Jan de Mooij [:jandem] from comment #1)
> Do you have a regression range?

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d74a9133f9a5c0a7374bef2a0be15e3ed7b37113&tochange=41d7a4864f3325cd9abcc5c5a17788334c8ca863

a11y is activated on my PC due to ATOK IME.
e10s off + a11y on(due to ATOK)  : fix, it is fast.
e10s on  + a11y off(Disable ATOK) : not fix.
e10s on  + a11y force off : not fix.


(It may not be related to JS. However, it is strange. Why does e10s affect to computing hash?).


Gecko profiler:
https://perfht.ml/2moFKf6
Blocks: 1310788
Component: JavaScript Engine → Disability Access APIs
Keywords: multiprocess
Summary: Performance regression, Computing hash on virus total is slow. Nightly54.0a1 is 5-10 times slower than Aurora53.0a2. → [e10s] Performance regression, Computing hash on virus total is slow. Nightly54.0a1 is 5-10 times slower than Aurora53.0a2.
Hm, it's probably unrelated to Aaron's patches but more of an e10s issue.

Milan, the profile (comment 2) shows a lot of time under PCompositorBridgeChild::OnMessageReceived. Any idea what's going on there?
Flags: needinfo?(milan)
(Reporter)

Comment 4

3 months ago
BTW, It is affected to 50+, if there is no add-on installed and no a11y application. Because, e10s is enabled by default.
status-firefox51: unaffected → wontfix
status-firefox52: unaffected → wontfix
status-firefox53: unaffected → ?
status-firefox-esr45: unaffected → disabled
status-firefox-esr52: unaffected → ?
(Reporter)

Comment 5

3 months ago
Created attachment 8841970 [details]
https://www.virustotal.com/static/js/sha256.js
(Reporter)

Comment 6

3 months ago
Created attachment 8841973 [details]
reduced.html
(Reporter)

Updated

3 months ago
Attachment #8841973 - Attachment description: reduced → reduced.html
(Reporter)

Comment 7

3 months ago
FYI, Bug 1342769 does not fix this issue.
(Reporter)

Comment 8

3 months ago
Okay, FileReaderSync.readAsBinaryString is 50 times slower in e10s.
Component: Disability Access APIs → DOM
Flags: needinfo?(milan) → needinfo?(amarchesini)
(Reporter)

Updated

3 months ago
Summary: [e10s] Performance regression, Computing hash on virus total is slow. Nightly54.0a1 is 5-10 times slower than Aurora53.0a2. → [e10s] Performance regression, Computing hash on virus total is slow. Nightly54.0a1 is 5-10 times slower than non e10s

Updated

3 months ago
status-firefox53: ? → affected

Comment 9

3 months ago
Track 54+ as performance regression.
tracking-firefox54: ? → +
(Reporter)

Updated

2 months ago
status-firefox55: --- → affected
Summary: [e10s] Performance regression, Computing hash on virus total is slow. Nightly54.0a1 is 5-10 times slower than non e10s → [e10s] Performance regression, Computing hash on virustotal is slow. Nightly54.0a1 is 5-10 times slower than non e10s
baku, needinfo ping?
Andrew, Nathan, this sounds potentially bad for 54. Should this be more of a priority to investigate? 
Looks like we have more detailed info in comment 8.
status-firefox53: affected → wontfix
Flags: needinfo?(overholt)
Flags: needinfo?(nfroyd)
(Assignee)

Comment 12

a month ago
Created attachment 8858552 [details] [diff] [review]
reader.patch

Here we are doing something odd. We are asking for a stream also when we have it, and then we ignore the new stream in SetStream.
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8858552 - Flags: review?(bugs)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #11)
> Andrew, Nathan, this sounds potentially bad for 54. Should this be more of a
> priority to investigate? 
> Looks like we have more detailed info in comment 8.

Andrea has a patch--thank you Andrea!  We should definitely try to get this uplifted sooner rather than later.
Flags: needinfo?(nfroyd)

Updated

a month ago
Attachment #8858552 - Flags: review?(bugs) → review+

Comment 14

a month ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7397d7229b32
PBlob should not create a remoteInputStream if that already exists, r=smaug
baku, please be sure to ask for uplift here.
Flags: needinfo?(overholt) → needinfo?(amarchesini)
(Assignee)

Comment 16

a month ago
Comment on attachment 8858552 [details] [diff] [review]
reader.patch

Approval Request Comment
[Feature/Bug causing the regression]: PBlob
[User impact if declined]: FileReader, when used in workers can be extremely slow.
[Is this code covered by automated tests?]: none
[Has the fix been verified in Nightly?]: manually, yes
[Needs manual test from QE? If yes, steps to reproduce]: yes, would be nice to have it tested.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: none
[Why is the change risky/not risky?]: instead recreating the stream each time we do it only onces. 
[String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8858552 - Flags: approval-mozilla-beta?
Attachment #8858552 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7397d7229b32
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Attachment #8858552 - Flags: approval-mozilla-aurora?
status-firefox-esr52: ? → affected
Hi Alice, 
can you help verify if this issue is fixed as expected?
Flags: needinfo?(alice0775)
(Reporter)

Comment 19

a month ago
I can reproduced the problem on Nightly55.0a1(2017-04-18)[1].
[1]https://hg.mozilla.org/mozilla-central/rev/bb38d935d699e0529f9e0bb35578d381026415c4
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 ID:20170418030220

And I verified as fixed on Nightly55.0a1(2017-04-19)[2].
[2]https://hg.mozilla.org/mozilla-central/rev/c0ea5ed7f91a6be996a4a3c5ab25e2cdf6b4377e
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 ID:20170419030223
Status: RESOLVED → VERIFIED
Flags: needinfo?(alice0775)
Comment on attachment 8858552 [details] [diff] [review]
reader.patch

Fix a perf regression and was verified. Beta54+. Should be in 54 beta 1.
Attachment #8858552 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 21

a month ago
uplift
https://hg.mozilla.org/releases/mozilla-beta/rev/cec324879dca - Andrea Marchesini - Bug 1343172 - PBlob should not create a remoteInputStream if that already exists, r=smaug. a=gchang
status-firefox54: affected → fixed
status-firefox55: fixed → verified
Flags: qe-verify+
Comment on attachment 8858552 [details] [diff] [review]
reader.patch

See comment 16.
Attachment #8858552 - Flags: approval-mozilla-esr52?
Comment on attachment 8858552 [details] [diff] [review]
reader.patch

User impact from comment 16 sounds severe, SWs, e10s are both new in ESR52, A+
Attachment #8858552 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+

Comment 24

17 days ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/c7b064f6b93a
status-firefox-esr52: affected → fixed
Reproduced the initial issue using old Nightly from 2017-02-28 on Windows 10, verified that the issue does not reproduce anymore using latest Firefox 54 beta 9 across platforms (Windows 10, macOS 10.12.4 and Ubuntu 16.04 32bit).
status-firefox54: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.