519624 - Consider a resolve hook for element.style

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I was just thinking...  This sort of coding pattern is reasonably common:

  var n = document.createElement("div");
  n.style.top = "12345px";
  n.style.left = "12345px";
  n.style.right = "12345px";
  n.style.bottom = "12345px";

The thing is, n.style is lazily allocated but thereafter immutable (except we possibly allow its jsobject to be collected).  Given that, in some ways the most natural way to implement n.style in jsapi is to have a resolve hook that defines the property (without any of the JSPROP_SHARED jazz, but readonly and with a stub getter).  That should have the incidental effect of repeated .style gets being really fast on trace assuming we hit the propcache, right?

Worth doing?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hrm.  For my testcase (which basically creates a bunch of nodes and sets style stuff on them, all in a loop, simulating someone creating various setups of lots of abs pos nodes), this loses at the very least because every time we get .style on a node for the first time, we branch exit off trace on this guard:

    About to try emitting guard code for SideExit=0x16996d4 exitType=BRANCH
    map = ld map[4]
    #0x3814 = int 14356
    guard_kshape = eq map, #0x3814
    ptr: xf guard_kshape -> pc=0x1cdbc67e imacpc=0x0 sp+0 rp+0 (GuardID=003)

I tried putting this:

   fprintf(stderr, "%p %p %d %d\n", entry->kpc, pc, entry->kshape, kshape_);

right before the equality tests in PROPERTY_CACHE_TEST, and the output looks something like this:

0x1cddf6d4 0x1cddf6cc 28549 28541
0x1cddf6d8 0x1cddf6d8 14526 14526
0x1cddf6e4 0x1cddf6e4 14526 14526
0x1cddf6f0 0x1cddf6f0 14526 14526
0x1cddf6d4 0x1cddf6cc 28549 28541
0x1cddf6d8 0x1cddf6d8 14526 14526
0x1cddf6e4 0x1cddf6e4 14526 14526
0x1cddf6f0 0x1cddf6f0 14526 14526

Do it looks like the .style lookup on that first line after creation consistently misses the property cache, ends up getting an atom back from js_FullTestPropertyCache because atom is not equal to entry->kpc.  In particular, atom is "style" while entry->kpc is in fact a pc.  Then we end up in js_FillPropertyCache and fill with the pc and the object's shape.

So the only real issue here seems to be this 28549 vs 28541 shape thing.

28541 seems to be the shape of the emptyScope of the divs' proto.  It's therefore the shape of the divs, which makes sense. But by the time we end up in js_FillPropertyCache the shape of the divs is 14526...

I guess the upshot is that there's no sane way we can have a resolve hook that actually defines a property on the object while staying on trace?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: The test patch I was using (including the printf from when I was trying to debug the perf regression)

Comment 3

[Ryan VanderMeulen [:RyanVM]]

bz, is this still wanted?

Comment 4

[Boris Zbarsky [:bzbarsky]]

No; it'd violate the spec for .style for one thing.