983686 - Implement parallel version of fromPar() for typed objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Niko Matsakis [:nmatsakis]]

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.

Comment 1

[Niko Matsakis [:nmatsakis]]

Attachment: Bug983686.diff

Simple version of fromPar. I think there is a way to structure the code nicer but this oughta work for now.

Comment 2

[Niko Matsakis [:nmatsakis]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=e453e32af4c1 (Green)

Comment 3

[Shu-yu Guo [: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

Comment 4

[Niko Matsakis [:nmatsakis]]

> 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".

Comment 5

[Niko Matsakis [:nmatsakis]]

Attachment: Bug983686.diff

Address nits from shu.

Comment 6

[Niko Matsakis [:nmatsakis]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12533335a2ed

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/12533335a2ed