The core of the existing mapPar code calls SetTypedObjectValue to assign a TypedObject returned from the kernel to the result array, but SetTypedObjectValue is not defined anywhere. (This probably highlights our need for a better test suite. I don't know if the function existed and was removed by accident, or never existed, but it's pretty clear the code is not being exercised at present.)

Hrm, using TypedObjectSet() instead of SetTypedObjectValue() yields correct results but a lot of parallel aborts, looks like that function was only used in sequential code previously.

Lars -- that's probably due to bug 977853, I landed some of the patches and TypedObjectSet() was removed. I do expect numerous parallel bailouts. I agree with need for better type suite, but the paths you are trodding are not (currently) expected to parallelize successfully (at least not by me).

Strange, TypedObjectSet is still in m-c. With one tweak TypedObjectSet may actually parallelize successfully (see ongoing discussion on #ionmonkey), at least for my test case; there's an issue with the type set that's being computed and a failure to inline. Anyway, this is a quasi-blocker for GC since I need to reinit the array to safely run the GC, which is why I've been going after it.

So the problem is that the UnsafeGetReservedSlot function does not get inlined because the type sets are imprecise due to a too-general type set for TypedObjectSet. Hannes says that we do not yet refine type sets at branches (he refers to bug 953164), and that the JIT does not know anything special about "IsObject" to let it do the refinement anyhow. In addition to that, the callee inherits the caller's type set, so a level of indirection does not work (tried it).

Patch that specializes TypedObjectSet to get more accurate type sets, for the one case where we need it the most. This appears to work, and does not cause bailouts. Since it uses memcpy - as did the code it's cloned from - I have some concerns about correctness in the face of fancy GC. Specifically, if the source object is in the nursery and the target object is tenured, and the type can contain references, this is wrong, because there would need to be a write barrier. Currently PJS ensures that that won't happen by running a minor GC before each parallel section (thus, no objects in the nursery). Since this code is only used from a parallel section it's fine. However, for the sequential maps the behavior of the original code is probably wrong. (Will investigate and file another bug if that is found to be the case.)

(In reply to Lars T Hansen [:lth] from comment #4) The bug tracking JIT work for refining type sets is bug 994016.

(In reply to Lars T Hansen [:lth] from comment #5) The bug tracking the Memcpy issue is bug 994018.

Comment on attachment 8403386 [details] [diff] [review] bug993396.patch Review of attachment 8403386 [details] [diff] [review]: ----------------------------------------------------------------- This doesn't have the desired semantics, I don't think. It requires that the user returns a typed object, rather than something adaptable to a typed object. What would probably solve your problem is to make TypedObjectSet() be callsite cloned, however.

Note that callsite cloning is a big hammer and I'm not sure it's approriate here. I'm also not sure why this is blocking your work on GC exactly -- you can rewrite tests not to go down this path fairly easily by using an out pointer. (Bug 933289 is another fix -- I have patches in progress but put them aside -- though it'll only work in the case where the user returns a consistent type.) That said, I imagine we can come with a more tailored workaround. I imagine something like an intrinsic like TestIsTypedObjectWithDescr(obj, descr). This would do something like: if (obj.isObject() && obj.toObject().is<TypedObject>() && obj.as<TypedObject>().descr().equiv(descr)) return obj; return null; and then in the self-hosted code we would do: var foo = TestIsTypedObjectWithDescr(input, descR); if (foo) { Memcpy(foo, ...); // or whatever } This way the type set for `foo` will be divorced from `input`. (Note that this adaptive path is not the perf sensitive path.)

(In reply to Niko Matsakis [:nmatsakis] from comment #9) > I'm also not sure why this is blocking your work on GC exactly -- you > can rewrite tests not to go down this path fairly easily by using an out > pointer. (Bug 933289 is another fix -- I have patches in progress but put > them aside -- though it'll only work in the case where the user returns a > consistent type.) (Also discussed on IRC) Well, "blocking". The problem is not this bug per se, but that the output buffer is not cleared after bailout, which is indeed blocking GC. (Bug 993347.)