Since switching to the new bullet implementation, I see a lot of time spent under nsBulletFrame::Ordinal.

For example:

• Loading https://en.wikipedia.org/wiki/Barack_Obama on my desktop, we spend 203 ms / 3089 ms = 6.6% of page load time in there: https://perfht.ml/2W6b5F1

• Loading https://html.spec.whatwg.org/ on my desktop, we spend 1625 ms / 10328 ms = 15.7% of page load time in there: https://perfht.ml/2WcvUP6

Do you know if we spend more time than before on the frame sorting code? I'm surprised that it being such a massive part of page load time according to your profiles it wasn't detected as a perf regression by Talos.

We should probably just stash the counter value on the bullet frame here, and make nsBulletFrame::Ordinal() O(1) again.

I think that if the marker uses content then that should already be handled by the SetText() call. Wdyt mats? I'm happy to write a patch for that.

Sounds good to me. Could you also remove the aDebugFromA11y param
we added in bug 1539656 too then? (essentially just revert that
patch modulo the #ifdef ACCESSIBILITY it added)

Alright, will do :)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79d29679e1fe7287173b5c92ef3d27e50a252c08

Now without broken stuff under it: https://treeherder.mozilla.org/#/jobs?repo=try&revision=89d7a6ce62d6b4c59824c7c13f39ee7bab00a876

[Triaging as a p3 regression from bug 205202, w/ status flags set accordingly. If any of my adjustments are wrong, apologies & please correct.]

(er, I'll bet this is more specifically a regression from bug 288704 " Make nsBulletFrame use the built-in 'list-item' CSS counter and remove the old implementation")

Err, the orange in that try run is because I forgot to update an assertion... Other than that it looks green, except for the nested pseudo-element case that my own test caught (dammit ;)).

I think there's some counters code assuming that nested pseudos don't exist. I haven't dug yet.

Third-time's the charm? https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=247434958&revision=e2017a11cc6437286730618249e28e8d16f1129a

I thought I was going to need it but turns out I don't. Still this is worth it I
think.

I'm going to need it to fix the counters code in presence of nested
pseudo-elements.

Depends on D31987

When you have a ::after::marker, and you compare one against the other we ended
up with the wrong result because of the pseudotype stuff.

I think this is cleaner now that DoCompareTreePosition handles pseudos properly
(which is really the thing this was working around).

Depends on D31988

I did this instead of just (ab)using the fact that every list item has at least
one counter-increment node because:

• I don't have the bullet frame around by the time we initially compute the
  counter increment, which means that I'd need to grow nsBlockFrame / add a
  frame property for the list item ordinal, which I think would be unfortunate.

• It feels more consistent with the way regular CSS counters work and with the
  way we want ::marker to eventually work.

Depends on D31989

https://hg.mozilla.org/mozilla-central/rev/d008903b2ace
https://hg.mozilla.org/mozilla-central/rev/e7dcf2a302c5
https://hg.mozilla.org/mozilla-central/rev/209e8a6ae29d
https://hg.mozilla.org/mozilla-central/rev/1a44a048e2f0
https://hg.mozilla.org/mozilla-central/rev/f023603f7e72

Comment on attachment 9066373 [details]
Bug 1552719 - Make nsBulletFrame::Ordinal() O(1) again. r=mats

Beta/Release Uplift Approval Request

• User impact if declined: Perf regression.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See bug 1544951 comment 0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively straight-forward patches, plus that part of the code is pretty well covered by automated tests.
• String changes made/needed: none

Hi Guys, I managed to reproduce this issue in Firefox 68.0b3 and there's a definite improvement in Firefox 69 but the blank page is still displayed for a bit while on Chrome its almost non existent. Can I mark this as Verified is the half a second of a blank page acceptable or it should be similar to Chrome ?

You should see whether Firefox 69 performs ~similar to Firefox 67. Firefox 68 should be slower than that because of the issue this patches fix.

Unfortunately no, Firefox 67 loads the page instantly, compared to that there's a visible difference, but compared to 68 its slightly improved.

So you're saying that 67 is much faster than 68, and still a bit faster than 69?

Please disregard my previous comment, I have downloaded the latest Nightly from https://tools.taskcluster.net/index/gecko.v2.mozilla-central.latest.firefox/win64-opt and the page loads Instantly, I will mark this issue accordingly.

Nice :)

Comment on attachment 9066371 [details]
Bug 1552719 - Make nsLayoutUtils::DoCompareTreePosition handle pseudos more diligently. r=mats,mattwoodrow

perf regression in 68, approved for 68.0b4

https://hg.mozilla.org/releases/mozilla-beta/rev/438492603ace
https://hg.mozilla.org/releases/mozilla-beta/rev/f3b0d67c8f66
https://hg.mozilla.org/releases/mozilla-beta/rev/7b115f244248
https://hg.mozilla.org/releases/mozilla-beta/rev/42530078a028

Hi, This issue is verified as fixed in our latest Beta 68.0b7. I will mark this issue accordingly.