Closed Bug 860965 Opened 7 years ago Closed 6 years ago

PJS: Move 1D ParallelArray operations to Array

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: shu, Assigned: shu)

References

Details

(Whiteboard: [PJS])

Attachments

(3 files, 1 obsolete file)

This is part of a bigger plan to split the current ParallelArray API into two: a new Matrix (ParallelMatrix)? constructor for higher-dimensioned computations, and migrating the existing 1D operations onto Array.prototype.

Pros:
  - Allows interleaving of parallel and sequential computation without changing data representation as ParallelArray would require.
  - Smaller API surface; more familiar ergonomics.
  - Lets us leverage existing engine optimizations for arrays without duplication. With ParallelArray being a different constructor, there'd be another []-path to optimize in all the JITs.

We do the following migration:

ParallelArray.prototype.map -> Array.prototype.pmap
ParallelArray.prototype.reduce -> Array.prototype.preduce
ParallelArray.prototype.scan -> Array.prototype.pscan
ParallelArray.prototype.scatter -> Array.prototype.pscatter
ParallelArray.prototype.filter -> Array.prototype.pfilter
Attached patch WIP (obsolete) — Splinter Review
The current WIP patch does not remove ParallelArray. The plan is such that until Matrix is ready, ParallelArray will remain. It refactors the following methods: map, reduce, scan, scatter, filter and shares the implementation of those between Array and ParallelArray.

The comprehension form is not refactored.
Assignee: general → shu
Attachment #736535 - Flags: feedback?(nmatsakis)
Attachment #736535 - Flags: review+
Comment on attachment 736535 [details] [diff] [review]
WIP

Carrying f+ from (presumably) accidental r+ from nmatsakis.

r? module owner for adding to Array.prototype.
Attachment #736535 - Flags: review?(luke)
Attachment #736535 - Flags: review+
Attachment #736535 - Flags: feedback?(nmatsakis)
Attachment #736535 - Flags: feedback+
Is this all behind an (about:config) pref?
Forgot to mention an important point: currently all PJS is behind ENABLE_PARALLEL_JS, which is only on for Nightly and Aurora, though we are thinking of turning it off even for Aurora.
Comment on attachment 736535 [details] [diff] [review]
WIP

Ah, I see.  Seems low-risk to inject these names then.
Attachment #736535 - Flags: review?(luke) → review+
Depends on: 844887
Blocks: PJS
Whiteboard: PJS
(These were being starred as bug 845190)
Depends on: 845190
In the new world, is there a way to construct an array in parallel using an elemental function?  I believe (new ParallelArray(10000, x => x)) runs in parallel.

Will there be a "pupdate" to modify an array in parallel in-place, or will arrays still be treated as immutable during all parallel operations?

What happens if you pass a non-conforming array to a parallel operation?  (e.g. an array with holes, or a sparse array, or an array with element getters.)

Will typed arrays (such as Int8Array) support these parallel operations?  Will there functions like pmap_to_Int8Array for *creating* typed arrays in parallel?
Blocks: 887989
Re-requesting review, since the strategy has changed since last review.

In the current strawman spec, the single-dimensioned ops on Array are going to be fairly different than the higher-dimensioned ops on Binary Data, such that I don't think refactoring them out as a separate file is worth the effort.

This patch copies the appropriate methods into Array.js.

The naming scheme has also changed to use a 'Par' suffix, per the strawman.

The ParallelArray implementation will remain in place until Binary Data is ready and we can move the higher-dimensioned operations there. We might rename it something like ParallelMatrix/Matrix in the interim once we implement the rest of the higher-dimensioned ops.
Attachment #736535 - Attachment is obsolete: true
Attachment #777433 - Flags: review?(nmatsakis)
Comment on attachment 777433 [details] [diff] [review]
Part 1: Copy parallel operations to Array

It is a lot of code to effectively duplicate. I wonder if we can find a way to share something later.
Attachment #777433 - Flags: review?(nmatsakis) → review+
Adds the sequential version of Array.build and some preliminary tests.
Attachment #777945 - Flags: review?(nmatsakis)
Tests that could be moved to use Arrays were moved to use Arrays. Other tests are *untouched* until we land Matrix, at which point we would transition them to Matrix (and later to BD). Fuzz bug tests (bugNNNNNN.js) are also untouched, but once ParallelArray is removed they should also be removed.

Also did some cleanup while I was at it:

 - Tests for Array methods are renamed to be prefixed with 'Array-methodName-'
 - Tests for features that should work in parallel are left as is.
 - Renamed the entire directory to 'parallel'
Attachment #777948 - Flags: review?(nmatsakis)
Forgot to mention that for part 3, I did not touch the scatter tests since it's uncertain how useful it is for JS arrays.
Attachment #777948 - Flags: review?(nmatsakis) → review+
Attachment #777945 - Flags: review?(nmatsakis) → review+
Depends on: 898750
No longer blocks: 887989
Depends on: 944975
Keywords: dev-doc-needed
Whiteboard: PJS → [PJS][DocArea=JS]
With bug 1117724 (Remove PJS), I guess that it makes no sense to document the PJS API on MDN at all for now.

Array.prototype.pmap
Array.prototype.preduce
Array.prototype.pscan
Array.prototype.pscatter
Array.prototype.pfilter

These are Nightly-only and are going to be removed. Please correct me if I'm wrong.
Keywords: dev-doc-needed
Whiteboard: [PJS][DocArea=JS] → [PJS]
(In reply to Florian Scholz [:fscholz] (MDN) from comment #17)
> With bug 1117724 (Remove PJS), I guess that it makes no sense to document
> the PJS API on MDN at all for now.

Right.

> Array.prototype.pmap
> Array.prototype.preduce
> Array.prototype.pscan
> Array.prototype.pscatter
> Array.prototype.pfilter
> 
> These are Nightly-only and are going to be removed. Please correct me if I'm
> wrong.

It is true that they are Nightly-only and will be removed.

However, they are not called that any longer (and haven't been called that for a while, at least since I started almost a year ago).  Instead they are mapPar, reducePar, scanPar, scatterPar, and filterPar.
You need to log in before you can comment on or make changes to this bug.