1330475 - Node.textContent is slow in some cases

Status: RESOLVED WORKSFORME

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

Description

[Olli Pettay [:smaug][[e-mail]]]

See https://bugzilla.mozilla.org/attachment.cgi?id=8826003

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

bz, curious, why we don't optimize out the whole loop in JS in this case?
  alert("Start");
  var e = document.getElementById("test")
  var Start = new Date().getTime();
  for (var i=0; i<50000000; i++) {
    var x = e.textContent;
  }
  alert((new Date().getTime() - Start));

textContent is marked as [Pure]

But other than that, it might make sense to convert the contents of a node to UTF16 when first time accessed from JS and then just share the string buffer when possible.

Comment 2

[Boris Zbarsky [:bzbarsky]]

It's marked [Pure], but also [Throws].  Unfortunately, throwing is observable.  This means that it can't be dead-code eliminated (because maybe it's supposed to have the side-effect of throwing) and cannot be moved across other things that might have side-effects, because then you could observe the order in which those things and the throw happen.  It _could_ still be loop-hoisted, maybe, if we made the assumption that the throwing is deterministic (because it's [Pure]), but it couldn't be hoisted out of _another_ loop, because it can't actually cross the side-effect of assigning i=0 in the loop header there.  But there's no way to communicate this to the JS engine...

The upshot is that we can't do any code-motion optimizations for attribute getters marked [Throws].  The [Pure] annotation _is_ still useful in that it lets other things be moved across this getter, by influencing its alias set.  But not as useful as it could be.

I guess in some sense that's a regression from bug 1037214, but I don't know that we have any seriously better options.  One possibly-better option would be to have an annotation for "throws, but only OOM", because that's not something script can rely on as a side-effect and is in any case no different from "OOM when converting from Gecko representation to JS representation", which _isn't_ something that prevents code motion.  Come to think of it, that's not a terrible idea.  I filed bug 1330536 on that.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

bug 1330536 made the testcase useless. Nightly takes now 100ms, Chrome 1800ms.
Need to write a new testcase which can't be optimzed out.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

That testcase ensures every iteration is run. And we seem to be faster than Chrome.
So, this could be marked as dup of bug 1330536 or WFM. WFM it is. Please reopen if needed.


(It might be interesting to see if bug 903519 changes anything here. Adding dependency just that someone might remember to re-test this later.)