web worker performance

RESOLVED FIXED in Firefox 54

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: gersh, Assigned: jandem)

Tracking

51 Branch
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 months ago
Created attachment 8841348 [details]
pi.html

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

I'm running Ubuntu 16.04.2 LTS on a 4x Intel(R) Core(TM) i5-4460S CPU @ 2.90GHz, and using pi.html (attached) to time the summing of a series expansion of pi.



Actual results:

With Chrome Version 56.0.2924.87 (64-bit) I get this output:

   points: 100000000 threads: 4
   serial pi: 3.1415926445762157 time: 0.5s  estimated MFlops: 1089.3
   map/reduce pi: 3.1415926445762157 time: 23.4s  estimated MFlops: 21.3
   parallel pi: 3.141592648326216 time: 0.5s  estimated MFlops: 3690.0

With Firefox 51.0.1 (64-bit) I get this output:

   points: 100000000 threads: 4
   serial pi: 3.1415926445762157 time: 0.4s  estimated MFlops: 1118.6
   map/reduce pi: 3.1415926445762157 time: 0.7s  estimated MFlops: 739.6
   parallel pi: 3.141592648326216 time: 1.7s  estimated MFlops: 1192.6



Expected results:

The parallel version spawning web workers is speeding up proportional to the number of cores in Chrome, but not Firefox.
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
(Assignee)

Updated

8 months ago
Flags: needinfo?(jdemooij)
(Assignee)

Comment 1

8 months ago
gersh, thanks for reporting this.

The issue seems to be that we don't have good type information in the parallel worker function. We probably end up with doubles instead of int32 values. If you change this:

  var points = evt.data.points
  var index = evt.data.index

To this:

  var points = evt.data.points|0
  var index = evt.data.index|0

It's way faster. I'll do some more digging.

Updated

8 months ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

8 months ago
Created attachment 8841572 [details] [diff] [review]
Patch

I'd ask Hannes to review but he's on PTO this week. In IonBuilder::maybeMarkEmpty we were looking at the instruction instead of its operand. The trivial fix speeds up the worker testcase.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8841572 - Flags: review?(bhackett1024)
Attachment #8841572 - Flags: review?(bhackett1024) → review+

Comment 3

8 months ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0c1e4792c8a
Fix IonBuilder::maybeMarkEmpty to look at the instruction's operands instead of the instruction itself. r=bhackett

Comment 4

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e0c1e4792c8a
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.