TI: Allow tracking of DOM instances in TypeObjects

RESOLVED FIXED in mozilla16



JavaScript Engine
5 years ago
5 years ago


(Reporter: efaust, Assigned: efaust)


Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [js:t])


(1 attachment)



5 years ago
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.


5 years ago
Blocks: 747288
No longer blocks: 747286
Whiteboard: [js:t]

Comment 1

5 years ago
Created attachment 636505 [details] [diff] [review]

Add DOM understanding to TI and stop marking all DOM prototypes and instances as UNKNOWN.
Attachment #636505 - Flags: review?(bhackett1024)
Comment on attachment 636505 [details] [diff] [review]

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. */

Attachment #636505 - Flags: review?(bhackett1024) → review+

Comment 3

5 years ago
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:

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

Comment 5

5 years ago
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

5 years ago

Comment 7

5 years ago
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Depends on: 774953
You need to log in before you can comment on or make changes to this bug.