1478138 - micro-optimize frame properties

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Nathan Froyd [:froydnj]]

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.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: micro-optimize frame properties

I verified that switching from ElementAt to indexing off of Elements removed
the array bounds checks.

Comment 2

[Daniel Holbert [: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

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5b992433341
micro-optimize frame property lookup; r=dholbert

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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).

Comment 5

[Nathan Froyd [:froydnj]]

(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?

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b5b992433341