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