1297805 - avoid array bounds checking in nsTArray methods

Status: NEW

(Core :: XPCOM, defect, P3)

Description

[Nathan Froyd [:froydnj]]

There are a few places in nsTArray where we know exactly how many
elements we're operating on, so we can avoid the bounds-checked
operator[] methods and use direct pointer accesses instead, which saves
space and time.avoid array bounds checking in nsTArray methods; r=erahm

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: avoid array bounds checking in nsTArray methods

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8784524 [details] [diff] [review]
avoid array bounds checking in nsTArray methods

Review of attachment 8784524 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/glue/nsTArray.h
@@ +2097,5 @@
>                              const char* aName,
>                              uint32_t aFlags = 0)
>  {
>    aFlags |= CycleCollectionEdgeNameArrayFlag;
>    size_t length = aField.Length();

You can get rid of |length|.

Comment 3

[Eric Rahm [:erahm]]

Comment on attachment 8784524 [details] [diff] [review]
avoid array bounds checking in nsTArray methods

Review of attachment 8784524 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Nathan, found this old review hanging around. Do you still want to land it?

::: xpcom/glue/nsTArray.h
@@ +2099,5 @@
>  {
>    aFlags |= CycleCollectionEdgeNameArrayFlag;
>    size_t length = aField.Length();
> +  for (auto& elem : aField) {
> +    ImplCycleCollectionTraverse(aCallback, elem, aName, aFlags);

Our iterator does bounds checking now, so this isn't really a win in this form.

Comment 4

[Nathan Froyd [:froydnj]]

Landing it would probably be good.  Do you have any objection to me making the obvious Elements()-related change to the cycle collection hook, since our iterators bounds-check now?

Comment 5

[Eric Rahm [:erahm]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> Landing it would probably be good.  Do you have any objection to me making
> the obvious Elements()-related change to the cycle collection hook, since
> our iterators bounds-check now?

r=me

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 7

[Andrew McCreight [:mccr8]]

Patch still looks relevant FWIW.