1394682 - Use template object in CreateIterResultObject

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1393712.

CreateIterResultObject is core part of async generator and it currently uses DefineProperty to define value and done on the result object,
but we could directly set native object slots, like RegExp match result object, to improve performance.

here's the performance improvement for the testcase in bug 1393712 comment #0, running with --no-async-stacks, with very rough patch applied:
  3410 [ms] -> 3151 [ms]

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: WIP: Use template object for iterator result object.

Comment 2

[Tooru Fujisawa [:arai]]

(now i'm moving template object to jscompartment)

main issue in the patch is, the type of "value" property.
I'm wondering if it's possible to make the template works on any type, but keep it optimizable based on actual value type.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: WIP: Use template object for iterator result object.

3503 [ms] => 3063 [ms]

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Use template object for iterator result object.

* Added NativeObject::createWithTemplate that creates native object from template (just like NewDenseFullyAllocatedArrayWithTemplate)
 * Added ConstraintTypeSet::generalize that adds unknown type

 * Added JSCompartment.iterResultTemplate_ that holds template object for iterator result object cache (sweeped on each GC)
 * Added JSCompartment::getOrCreateIterResultTemplateObject that creates the template object
   with `value` and `done` properties on plain object, with generalized type for `value` property
 * Changed CreateIterResultObject to create the object from template,
   and set properties with setSlot to improve performance

Comment 5

[Kannan Vijayan [:djvj]]

Comment on attachment 8904773 [details] [diff] [review]
Use template object for iterator result object.

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

Nice job!  No issues aside from a couple of nits.

::: js/src/jsiter.cpp
@@ +57,5 @@
>  
>  static const gc::AllocKind ITERATOR_FINALIZE_KIND = gc::AllocKind::OBJECT2_BACKGROUND;
>  
> +static const size_t IterResultObjectValueSlot = 0;
> +static const size_t IterResultObjectDoneSlot = 1;

It might be better to keep these variables close to the actual template object declaration in jscompartment.h

::: js/src/vm/TypeInference.h
@@ +700,5 @@
>       */
>      void addType(JSContext* cx, Type type);
>  
> +    /* Generalize to any type. */
> +    void generalize(JSContext* cx) {

"generalize" seems too vague a name for this.  I'd prefer it be more explicit, like "makeUnknown()".

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c86474c75be
Use template object for iterator result object. r=djvj

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c86474c75be02a4d568a33bce49d31bbbf88fa5
Bug 1394682 - Use template object for iterator result object. r=djvj

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4c86474c75be