Closed Bug 1330226 Opened 4 years ago Closed 4 years ago

BidiParagraphData is rather alloc heavy

Categories

(Core :: Layout: Text and Fonts, defect)

50 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: smaug, Assigned: smaug)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Attachment #8825671 - Flags: review?(dholbert)
Attachment #8825671 - Attachment is obsolete: true
Attachment #8825671 - Flags: review?(dholbert)
Attachment #8825674 - Flags: review?(dholbert)
Comment on attachment 8825674 [details] [diff] [review]
bidi_less_malloc.diff

This looks OK to me, but it's really more in jfkthame's wheelhouse -- punting to him.

(The one thing I'm less sure about here is whether 16 might be too large for the two newly-converted AutoTArrays here... I don't have a good feel for typical usage of this object, so I don't know how often we actually hit that many.  I wonder if 4 or 8 would be a better default size there [with failover to the heap in cases where that's exceeded of course].)

Anyway, I'll punt to jfkthame.
Attachment #8825674 - Flags: review?(dholbert) → review?(jfkthame)
I noticed the allocations when profiling a google doc and there the number of items was most often 1-4, occasionally up to 16 then rarely over 20.
Happy to use lower number too.
Comment on attachment 8825674 [details] [diff] [review]
bidi_less_malloc.diff

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

LGTM, thanks. 16 seems a reasonable size for the auto-arrays, IMO; that should be enough that heap allocation here becomes pretty rare, though some complex content will still need it.
Attachment #8825674 - Flags: review?(jfkthame) → review+
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0733a44cd6c5
BidiParagraphData should allocate less, r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/0733a44cd6c5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Assignee: nobody → bugs
You need to log in before you can comment on or make changes to this bug.