766447 - TI: Allow tracking of DOM instances in TypeObjects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Eric Faust [:efaust]]

We need to add a flag to TypeObjects to track whether the objects of that type are DOM instances. It will behave similarly to OBJECT_FLAG_NON_TYPED_ARRAY.

Comment 1

[Eric Faust [:efaust]]

Attachment: Patch

Add DOM understanding to TI and stop marking all DOM prototypes and instances as UNKNOWN.

Comment 2

[Brian Hackett [Laid off!]]

Comment on attachment 636505 [details] [diff] [review]
Patch

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

::: js/src/jsapi.h
@@ +3647,5 @@
>  #define JSCLASS_HAS_PRIVATE             (1<<0)  /* objects have private slot */
>  #define JSCLASS_NEW_ENUMERATE           (1<<1)  /* has JSNewEnumerateOp hook */
>  #define JSCLASS_NEW_RESOLVE             (1<<2)  /* has JSNewResolveOp hook */
>  #define JSCLASS_PRIVATE_IS_NSISUPPORTS  (1<<3)  /* private is (nsISupports *) */
> +#define JSCLASS_IS_DOMJSCLASS           (1<<4)  /* objects are DOM */

This might be better off in jsfriendapi.h

::: js/src/jsinfer.cpp
@@ +1893,5 @@
>      new(object) TypeObject(proto, key == JSProto_Function, unknown);
>  
>      if (!cx->typeInferenceEnabled())
>          object->flags |= OBJECT_FLAG_UNKNOWN_MASK;
> +    else {

'if' needs {} to match the else

@@ +5709,5 @@
>      if (!table.initialized() && !table.init())
>          return NULL;
>  
>      TypeObjectSet::AddPtr p = table.lookupForAdd(this);
>      if (p) {

In the case where the lookup is successful, this needs to make sure to set the OBJECT_FLAG_NON_DOM bit on the object if !isDOM, as it's currently possible to have different objects that are both DOM and non-DOM and have the same type.

::: js/src/jsinfer.h
@@ +292,5 @@
>      /* For a global object, whether flags were set on the RegExpStatics. */
>      OBJECT_FLAG_REGEXP_FLAGS_SET      = 0x00800000,
>  
> +    /* Whether any objects this represents are not DOM objects. */
> +    OBJECT_FLAG_NON_DOM               = 0x01000000,

It would be nice if this flag went after OBJECT_FLAG_NON_TYPED_ARRAY (and the values for the other flags shuffled a bit), as that group is the one this is logically related to.

::: js/src/jsproxy.cpp
@@ +1767,5 @@
>  
>      /* Don't track types of properties of proxies. */
>      MarkTypeObjectUnknownProperties(cx, obj->type());
>  
> +    /* Set the new proxy as having a singleton type. */

s/Set/Mark/

Comment 3

[Ed Morley [:emorley]]

Unfortunately bug 772303 (https://hg.mozilla.org/integration/mozilla-inbound/rev/fad7d06d7dd5) had to be backed out for winxp pgo-only jsreftest failures.

This bug (amongst others) either caused unresolvable (for someone who doesn't work on the JS engine at least) conflicts with the backout of fad7d06d7dd5, or else bugzilla dependencies indicated that one of it's dependants had now been backed out. 

Since there was no one in #developers that could resolve the conflicts, unfortunately this bug has been backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0be4b70b814

Comment 4

[Matt Brubeck (:mbrubeck)]

This bug also caused regressions in Dromaeo (DOM) and Trace Malloc MaxHeap benchmarks.  Was that expected?

Comment 5

[Eric Faust [:efaust]]

I think so, if only a tiny bit. We had to change the way TI marked some newly created objects, in particular proxies, that may lead to small increases in allocations. It shouldn't be much of a hit, though, and it moves towards JIT integration of DOM methods and property accesses.

Comment 6

[Eric Faust [:efaust]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b2fb5e208e4

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9b2fb5e208e4