Closed Bug 983686 Opened 9 years ago Closed 9 years ago

Implement parallel version of fromPar() for typed objects

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: nmatsakis, Assigned: nmatsakis)

References

Details

Attachments

(1 file, 1 obsolete file)

We currently have a parallel mapPar() but not fromPar(). They are almost the same and in fact the code is already designed to handle it.
Blocks: PJS
Blocks: 966567
Attached patch Bug983686.diff (obsolete) — Splinter Review
Simple version of fromPar. I think there is a way to structure the code nicer but this oughta work for now.
Attachment #8391281 - Flags: review?(shu)
Comment on attachment 8391281 [details] [diff] [review]
Bug983686.diff

Review of attachment 8391281 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Some style nits, typos, and testing infrastructure questions.

::: js/src/builtin/TypedObject.js
@@ +927,5 @@
>    }
>  }
>  
>  // Warning: user exposed!
> +function TypedArrayMap(a,b) {

Looks like you accidentally deleted a space. Also seems like the spacing is inconsistent throughout the file for function arguments. I'd like everything to have spaces, but it doesn't have to be this bug.

@@ +959,2 @@
>      return MapTypedParImpl(this, a, thisType, b);
> +  else if (IsCallable(a))

else after return

@@ +1032,5 @@
> +  // Detect whether an explicit depth is supplied.
> +  if (typeof b === "number" && IsCallable(c))
> +    return MapTypedParImpl(a, b, this, c);
> +  else if (IsCallable(b))
> +    return MapTypedParImpl(a, 1, this, b);

else after return

::: js/src/jit-test/tests/parallel/TypedObj-fromPar-outpointer-struct.js
@@ +1,1 @@
> +// Test basic mapPar parallel execution using an out

Typo: mapPar -> fromPar

@@ +1,4 @@
> +// Test basic mapPar parallel execution using an out
> +// pointer to generate a struct return.
> +
> +if (!this.hasOwnProperty("TypedObject"))

Does having TypedObject => having PJS? Seems a sequential version of TypedObjects alone is very useful.

::: js/src/jit-test/tests/parallel/TypedObj-fromPar-return-scalar.js
@@ +1,1 @@
> +// Test basic mapPar parallel execution.

Ditto
Attachment #8391281 - Flags: review?(shu) → review+
> Does having TypedObject => having PJS? Seems a sequential version of
> TypedObjects alone is very useful.

The right thing to test is getBuildConfiguration().parallelJS(), actually...I should probably make one for typed objects too and stop using this hokey test. For one thing I'd like to rename the TypedObject "module".
Attached patch Bug983686.diffSplinter Review
Address nits from shu.
Attachment #8391281 - Attachment is obsolete: true
Attachment #8393020 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/12533335a2ed
Assignee: nobody → nmatsakis
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.