Closed Bug 1371277 Opened 7 years ago Closed 7 years ago

Use reserved space for one item in SelectionState::mArray

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This avoids dynamic allocations in SelectionState::SaveSelection() in the common
case for the selections that only have a maximum of one range stored in them.
I noticed this while profiling speedometer's VanillaJS test.  This shows up under the Placeholder transaction stuff from input.value setter code, and this patch completely removed SelectionState::SaveSelection() from the profile by eliminating the need for dynamic allocations there.
Attachment #8875724 - Flags: review?(masayuki)
Blocks: 1346723
Assignee: nobody → ehsan
Comment on attachment 8875724 [details] [diff] [review]
Use reserved space for one item in SelectionState::mArray

Nice!

I have a question. In struct/class which are used only in stack, nsTArray should be always replaced with AutoTArray? (Although I don't know if there are such structs/classes.)
Attachment #8875724 - Flags: review?(masayuki) → review+
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #2)
> I have a question. In struct/class which are used only in stack, nsTArray
> should be always replaced with AutoTArray? (Although I don't know if there
> are such structs/classes.)

There are no hard rules about it AFAIK.  We usually use AutoTArray in the situations where we expect the array to have a common number of elements and we'd like to avoid dynamic memory allocation cost and we don't care about the additional memory usage.  Using additional stack space is *usually* OK unless if your code runs under our recursive algorithms such as reflow where we actually may run the risk of running out of stack space.  Otherwise allocating even relatively large buffers on the stack is OK and is blazingly fast (well, "allocating" the space is pretty much adding an offset to the stack pointer register and potentially some book keeping that the compiler will do for us).
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7680538080d1
Use reserved space for one item in SelectionState::mArray; r=masayuki
https://hg.mozilla.org/mozilla-central/rev/7680538080d1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: