516857 - Setting expando properties on DOM nodes falls off trace

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Testcase is basically:

      for (var i = 0; i < 10; ++i) {
        var d = document.createElement("div");
        d.test = "test"+i;
      }

I get:

  abort: 10042: can't trace uncacheable property set
  Abort recording of tree file:///Users/bzbarsky/baz.html:5@14 at
    file:///Users/bzbarsky/baz.html:7@41: SetPropHit.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note to self: if |d| is outside the loop, the abort is different; may want a different bug there.

Comment 2

[Boris Zbarsky [:bzbarsky]]

So what happens is that we land in js_FillPropertyCache.  cs->format includes JSOF_SET. |adding| is true, sprop == scope->lastProp and scope->shape == sprop->shape, so we enter that branch.  sprop->parent is null.  So we look up the proto, and then the protoscope.  protoscope->emptyScope is null, so we return JS_NO_PROP_CACHE_FILL.

The sprop is more or less what one would expect here: its id is "test" and it has:

  getter = <XPC_WN_Helper_GetProperty(JSContext*, JSObject*, long, long*)>
  setter = <XPC_WN_Helper_SetProperty(JSContext*, JSObject*, long, long*)>

since those are the class get/set property hooks for XPConnectified objects.

proto is of class XPC_WN_ModsAllowed_NoCall_Proto_JSClass.

In particular, the JSClass of proto is NOT the same as the JSClass of obj (the latter is HTMLDivElement).  So js_ObjectIsSimilarToProto is false in this case (and hence InitScopeForObject never called getEmptyScope and emptyScope is null).

In fact, the two classes have different flags, different getObjectOps, etc.

So it looks like the _first_ expando property we set on any xpconnectified object will always be a property cache miss, and hence a trace abort.  The original testcase I ran into this on created a bunch of DOM nodes in a loop and set a bunch of properties on them; the loop didn't trace, of course.

Any ideas how we can make this better, either in jseng or in xpconnect?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Possible regression from bug 503080.

Comment 4

[Peter Van der Beken [:peterv]]

(In reply to comment #2)
> In particular, the JSClass of proto is NOT the same as the JSClass of obj (the
> latter is HTMLDivElement).  So js_ObjectIsSimilarToProto is false in this case
> (and hence InitScopeForObject never called getEmptyScope and emptyScope is
> null).

This is after bug 505523, right? I think before that patch js_ObjectIsSimilarToProto would have returned true in this case.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er, yes.  It is.  That makes me feel better about the fact that we just noticed the regression....

Comment 6

[Boris Zbarsky [:bzbarsky]]

Hmm.  The XPConnect behavior of changing classes when switching from slimwrapper to XPCWrappedNative that Peter mentions in bug 505523 might be a problem for the solution Jason and I discussed of branding the emptyScope with the child's class.  Are there things that would cause the change but leave the child with an empty scope?  What things actually cause a conversion nowadays?

Comment 7

[Boris Zbarsky [:bzbarsky]]

We no longer switch classes dynamically for xpconnect.  So we should revisit this, but probably as part of bug 622298.

Comment 8

[Andreas Gal :gal]

The common case (same origin) has no wrapper in between any more. The uncommon case is always off trace right now (since wrappers are non-native).

Comment 9

[Boris Zbarsky [:bzbarsky]]

I'm not sure what wrappers have to do with anything.  They weren't involved in comment 2...

In the bug this blocks, removing the expando sets speeds up a DOM loop creating nodes by 2x

Comment 10

[Boris Zbarsky [:bzbarsky]]

No longer relevant now that the tracer is gone.