Closed
Bug 1343172
Opened 8 years ago
Closed 8 years ago
[e10s] Performance regression, Computing hash on virustotal is slow. Nightly54.0a1 is 5-10 times slower than non e10s
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: alice0775, Assigned: baku)
References
Details
(Keywords: multiprocess, perf, regression)
Attachments
(3 files)
6.17 KB,
application/x-javascript
|
Details | |
602 bytes,
text/html
|
Details | |
650 bytes,
patch
|
smaug
:
review+
gchang
:
approval-mozilla-beta+
ritu
:
approval-mozilla-esr52+
|
Details | Diff | Splinter Review |
[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
Comment 1•8 years ago
|
||
Do you have a regression range?
Reporter | ||
Comment 2•8 years 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.
Comment 3•8 years ago
|
||
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•8 years ago
|
||
BTW, It is affected to 50+, if there is no add-on installed and no a11y application. Because, e10s is enabled by default.
Reporter | ||
Comment 5•8 years ago
|
||
Reporter | ||
Comment 6•8 years ago
|
||
Reporter | ||
Updated•8 years ago
|
Attachment #8841973 -
Attachment description: reduced → reduced.html
Reporter | ||
Comment 7•8 years ago
|
||
FYI, Bug 1342769 does not fix this issue.
Reporter | ||
Comment 8•8 years ago
|
||
Okay, FileReaderSync.readAsBinaryString is 50 times slower in e10s.
Component: Disability Access APIs → DOM
Updated•8 years ago
|
Flags: needinfo?(milan) → needinfo?(amarchesini)
Reporter | ||
Updated•8 years 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•8 years ago
|
Reporter | ||
Updated•8 years 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
Comment 10•8 years ago
|
||
baku, needinfo ping?
Comment 11•8 years ago
|
||
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.
Assignee | ||
Comment 12•8 years ago
|
||
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)
Comment 13•8 years ago
|
||
(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•8 years ago
|
Attachment #8858552 -
Flags: review?(bugs) → review+
Comment 14•8 years 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
Comment 15•8 years ago
|
||
baku, please be sure to ask for uplift here.
Flags: needinfo?(overholt) → needinfo?(amarchesini)
Assignee | ||
Comment 16•8 years 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?
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7397d7229b32
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•8 years ago
|
Attachment #8858552 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
Comment 18•8 years ago
|
||
Hi Alice, can you help verify if this issue is fixed as expected?
Flags: needinfo?(alice0775)
Reporter | ||
Comment 19•8 years 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 20•8 years ago
|
||
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•8 years 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
Updated•8 years ago
|
Flags: qe-verify+
Comment 22•8 years ago
|
||
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•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/c7b064f6b93a
Comment 25•7 years ago
|
||
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).
Flags: qe-verify+
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•