Closed Bug 1478138 Opened Last year Closed Last year

micro-optimize frame properties

Categories

(Core :: Layout, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file)

Once we've determined an index for the frame property, we don't need to
pay the overhead of checking whether that index is in bounds for the
array again on the next access.  It would be nice if we could avoid that
overhead for removal, too, but since there's no API for that, we'll just
remove the overhead for accesses.
I verified that switching from ElementAt to indexing off of Elements removed
the array bounds checks.
Attachment #8994653 - Flags: review?(dholbert)
Comment on attachment 8994653 [details] [diff] [review]
micro-optimize frame properties

Commit message nit:
> micro-optimize frame properties

Maybe "frame property lookups", to be a little more specific about what's being optimized here (lookup time, vs. optimizing storage / something else)



r=me
Attachment #8994653 - Flags: review?(dholbert) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5b992433341
micro-optimize frame property lookup; r=dholbert
Idea:
Instead of doing an IndexOf (which goes through the elements already, and calculates an index from a pointer) followed by operator[] (which needs to recalculate the pointer from the index), why not have a search function that returns a pointer to the element or nullptr?
A less scary option could be a function that takes a lambda and runs it on the reference of the found element (if any).
(In reply to Gerald Squelart [:gerald] from comment #4)
> Idea:
> Instead of doing an IndexOf (which goes through the elements already, and
> calculates an index from a pointer) followed by operator[] (which needs to
> recalculate the pointer from the index), why not have a search function that
> returns a pointer to the element or nullptr?
> A less scary option could be a function that takes a lambda and runs it on
> the reference of the found element (if any).

The frame property manipulation functions indicate that we're missing some kind of interface: both RemoveInternal and DeleteInternal are needlessly inefficient, because the RemoveElementAt will do unnecessary bounds checks.  I'm not quite sure what the right interface is, though.  For those cases, if you returned a pointer to the element, or even ran a lambda on the found element, you'd need to add a RemoveElementByPointer or something, which is very sketchy.

For RemoveInternal, at least, some RemoveElement-style function that returns a Maybe<T> would be about right.  I guess that would work for DeleteInternal, too?
https://hg.mozilla.org/mozilla-central/rev/b5b992433341
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.