As noted in bug 1409047 comment 9, the code that does PaintRowGroupBackgroundByColIdx in DisplayGenericTablePart makes us walk all cells in the table for every single column. This is somewhat bad. Maybe bug 1352499 will help enough here. Maybe not. What we should really consider doing is as we walk the columns we should keep state as to where we stopped the walk over cells in each row and start there instead of walking the entire row (or the row up until the end of the col index range, at least; I will be filing bugs blocking bug 1409047 to do that last bit, at least).
2 years ago
Morris, do you have time to look into this?
2 years ago
Oh, also, the fact that we OrderRowGroups() separately for each columns is annoying too; I wish we could avoid that.
Sorry for late reply. I'll look into this later. Anyone who interested in this can steal it.
Look at the number from bug 1409047 comment 20, the perf becomes much better. I do agree it's still worth to fix this bug and will try to find people to work on this
Fundamentally, our current table painting behavior is O(N*N*M) where N is number of columns and M is number of rows. It really needs to be O(N*M); that's what this bug is about. In some cases we may be able to paint tables less and hence have this problem not bite very much, but those are all band-aids.... :(
We still expect to end up with the same number of display items, this is *just* a question of producing those display items with a better O(), right?
That is correct. We end up with O(N*M) display items, because that's how many cells we have. No surprise there. The bad part is doing that much work for every single column.
Flags: needinfo?(howareyou322) → needinfo?(dbaron)
Assignee: nobody → matt.woodrow
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/a8bd87bfc775 Part 1: Remove current table item, as it's never set. r=miko https://hg.mozilla.org/integration/autoland/rev/c0aa0d405e3c Part 2: Get rid of generic table painting code, and handle each class separately. r=dbaron https://hg.mozilla.org/integration/autoland/rev/1782b76bbc22 Part 3: Add helpers for box shadow creation. r=miko https://hg.mozilla.org/integration/autoland/rev/5137a23cfbba Part 4: Hoist outline skipping into col(group) frame code. r=dbaron https://hg.mozilla.org/integration/autoland/rev/34651ebcbaf4 Part 5: Skip box-shadow for table column and column groups. r=dbaron https://hg.mozilla.org/integration/autoland/rev/e88eae3b48a7 Part 6: Store column and column group backgrounds separately, and then append then before the rest of the table contents. r=dbaron https://hg.mozilla.org/integration/autoland/rev/df9a4342bf97 Part 7: Pass rects in display list coordinates to AppendBackgroundItemsToTop. r=miko https://hg.mozilla.org/integration/autoland/rev/69b049f54622 Part 8: Create column and column group background display items as part of the cell's BuildDisplayList. r=dbaron https://hg.mozilla.org/integration/autoland/rev/1c9357112b30 Part 9: Used cached values instead of calling nsDisplayListBuilder::ToReferenceFrame when possible, since it can be expensive when the requested frame isn't the builder's current frame. r=miko https://hg.mozilla.org/integration/autoland/rev/1ca7d32474c6 Part 10: Make sure we build display items for table parts where only the normal position is visible, since we may need to create background items for ancestors at that position. r=dbaron https://hg.mozilla.org/integration/autoland/rev/2543d1e28c30 Part 11: Create an AutoBuildingDisplayList when we create background items for table columns and column groups, so that we initialize the invalidation state correctly. r=kamidphish
You need to log in before you can comment on or make changes to this bug.