Closed
Bug 1445229
Opened 7 years ago
Closed 7 years ago
[css-grid] Store the number of grid items per span-length to avoid iterating them again
Categories
(Core :: Layout, enhancement, P3)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
Details
Attachments
(1 file, 1 obsolete file)
Just a little optimization on top of bug 1425599.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8958416 -
Flags: review?(dholbert)
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8958416 -
Attachment is obsolete: true
Attachment #8958416 -
Flags: review?(dholbert)
Attachment #8958421 -
Flags: review?(dholbert)
Comment 3•7 years ago
|
||
Comment on attachment 8958421 [details] [diff] [review]
[css-grid] Store the number of grid items per span-length to avoid iterating them again (idempotent change)
Review of attachment 8958421 [details] [diff] [review]:
-----------------------------------------------------------------
r=me, one optional nit:
::: layout/generic/nsGridContainerFrame.cpp
@@ +4360,4 @@
> uint32_t len = 2 * span;
> + perSpanData.SetCapacity(len);
> + for (uint32_t i = perSpanData.Length(); i < len; ++i) {
> + perSpanData.AppendElement(PerSpanData{0, TrackSize::StateBits(0)});
If you gave PerSpanData a default constructor with ": mItemCount(0), mStateBits(0)" (somewhat useful on its own merits as an uninitialized-data foolproofing measure), then you could replace this whole "if" clause with a single nsTArray::SetLength() call, like so:
if (span >= perSpanData.Length()) {
perSpanData.SetLength(2 * span);
}
(Because nsTArray::SetLength automatically invokes the default constructor for new elements that it appends.)
Attachment #8958421 -
Flags: review?(dholbert) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/023c4f7e7d76
[css-grid] Store the number of grid items per span-length to avoid iterating them again (idempotent change). r=dholbert
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #3)
> if (span >= perSpanData.Length()) {
> perSpanData.SetLength(2 * span);
Yeah, that's much better, thanks!
Assignee | ||
Updated•7 years ago
|
Flags: in-testsuite-
Comment 6•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•