Note: There are a few cases of duplicates in user autocompletion which are being worked on.

make nsPresArena and display list arena use infallible allocation

RESOLVED FIXED in Firefox 18

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

(Blocks: 1 bug)

Trunk
mozilla19
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox18 fixed)

Details

Attachments

(5 attachments)

(Assignee)

Description

5 years ago
We should make nsPresArena and the display list arena be infallible allocators; they allocate memory in small pieces and many of the existing callers don't handle out-of-memory.  So they should just abort on allocation failure.
(Assignee)

Comment 1

5 years ago
Created attachment 679249 [details] [diff] [review]
, patch 1:  Make allocation in the frame arena and the display list arena infallible.
Attachment #679249 - Flags: review?(roc)
(Assignee)

Comment 2

5 years ago
Created attachment 679250 [details] [diff] [review]
, patch 2:  Don't null-check inside NS_New*Frame.
Attachment #679250 - Flags: review?(roc)
(Assignee)

Comment 3

5 years ago
Created attachment 679251 [details] [diff] [review]
, patch 3:  Stop handling allocation failures in the style system that no longer need to be handled.
Attachment #679251 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

5 years ago
Created attachment 679252 [details] [diff] [review]
, patch 4:  Remove null-checks of NS_New*Frame callers in the frame constructor.
Attachment #679252 - Flags: review?(roc)
(Assignee)

Updated

5 years ago
Attachment #679252 - Flags: review?(roc) → review?(bzbarsky)
Attachment #679249 - Flags: review?(roc) → review+
Attachment #679250 - Flags: review?(roc) → review+
You should remove:

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsIPresShell.h#204
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsIPresShell.h#232

too.
(Assignee)

Comment 6

5 years ago
Landed patches 1 and 2 ahead of bug 572200:
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6e5f31366aab
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cf16f2f84efc
Whiteboard: [leave open]
(Assignee)

Comment 7

5 years ago
Created attachment 679307 [details] [diff] [review]
, patch 5:  Remove unneeded null-checks in nsIPresShell.h.
Attachment #679307 - Flags: review?(roc)
Attachment #679307 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/6e5f31366aab
https://hg.mozilla.org/mozilla-central/rev/cf16f2f84efc

Comment 9

5 years ago
Comment on attachment 679251 [details] [diff] [review]
, patch 3:  Stop handling allocation failures in the style system that no longer need to be handled.

r=me.  Nice to see all that code go away!
Attachment #679251 - Flags: review?(bzbarsky) → review+

Comment 10

5 years ago
Comment on attachment 679252 [details] [diff] [review]
, patch 4:  Remove null-checks of NS_New*Frame callers in the frame constructor.

>+      // XXXldb Can we remove this now?

Yes.

r=me, but I bet there are lots of methods returning nsresult that can stop doing that now... Maybe file a followup to clean that up?
Attachment #679252 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 11

5 years ago
Landed patch 3 through patch 5:
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c6e38e325158
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0be0f375cac8
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d6c8276fe88e


Note that patch 5 addresses comment 5.
Whiteboard: [leave open]
(Assignee)

Comment 12

5 years ago
Comment on attachment 679249 [details] [diff] [review]
, patch 1:  Make allocation in the frame arena and the display list arena infallible.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Unable to land significant B2G performance improvement in bug 572200 without potentially risky logic changes.
Testing completed (on m-c, etc.): on mozilla-central
Risk to taking this patch (and alternatives if risky): The risk should be low; it's just applying the concept of infallible memory allocations which we already use extensively to one new class of small memory allocations.
String or UUID changes made by this patch: None.
Attachment #679249 - Flags: approval-mozilla-aurora?
Comment on attachment 679249 [details] [diff] [review]
, patch 1:  Make allocation in the frame arena and the display list arena infallible.

Approving on aurora as I understand it helps with b2g performance
Attachment #679249 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/c6e38e325158
https://hg.mozilla.org/mozilla-central/rev/0be0f375cac8
https://hg.mozilla.org/mozilla-central/rev/d6c8276fe88e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/fae3b9538755
status-firefox18: --- → fixed
Depends on: 820220
No longer depends on: 820220
(Assignee)

Updated

4 years ago
Blocks: 856879
Blocks: 1171282
You need to log in before you can comment on or make changes to this bug.