Closed Bug 944155 Opened 11 years ago Closed 6 years ago

Build b2g with --disable-xul

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
All
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: ehsan.akhgari, Assigned: fabrice)

Details

Attachments

(4 files)

no-xul.patch
77.62 KB, patch
Details | Diff | Splinter Review
less-xul.patch
41.06 KB, patch
Details | Diff | Splinter Review
memory-reports, xul enabled
685.55 KB, text/plain
Details
memory-reports, xul disabled
668.89 KB, text/plain
Details 
Technically, we should not have any dependencies on XUL.  It would be nice if we could avoid building and shipping this code.

Michael, any idea who the right person to work on this would be?  (And what the right component for this bug would be?)
Flags: needinfo?(mwu)
I could try to build it tonight and see what problem we run into.
(In reply to comment #1)
> I could try to build it tonight and see what problem we run into.

That would be great, thanks!
I started on that when working on bug 894927. I guess I could rebase my patch...

Things that I'm sure are broken when removing xul support:
- scrollbars
- html <menu> (it uses <xul:menu>)
- video and audio controls
(In reply to Fabrice Desré [:fabrice] from comment #3)
> I started on that when working on bug 894927. I guess I could rebase my
> patch...
> 
> Things that I'm sure are broken when removing xul support:
> - scrollbars
> - html <menu> (it uses <xul:menu>)

Sigh :(

> - video and audio controls

I thought they just use XBL?
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)

> > - video and audio controls
> 
> I thought they just use XBL?

They use xbl, but the controls themselves are xul elements. I'm fairly sure we could move that to html since we now have a nice flex model available here also.
(In reply to comment #5)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)
> 
> > > - video and audio controls
> > 
> > I thought they just use XBL?
> 
> They use xbl, but the controls themselves are xul elements.

Yes, you're right.

> I'm fairly sure we
> could move that to html since we now have a nice flex model available here
> also.

Yeah, I think we should be able to do that too.
The scrollbars I see in gaia are done in JS, are you sure they need XUL? Sounds like it's not much to fix. How much code would that save us? It's particularly bad if we -barely- use it because we have to map these pages into memory but could avoid using it with little effort.
Not sure what you mean by "done in js". They are "standard" scrollbars that use xul:thumb and xul:scrollbar. We style them in https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/content.css#13

Once we'll apzc everywhere I think the responsibility of drawing scrollbars should be at the layout/gfx level. So no big deal for this bug indeed.

I'll post what I have later today once I get everything building again.
Assignee: nobody → fabrice
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → All
Any idea how much the memory saving we could gain here?
From Bug 894927 comment 26, I think we can save at least 1~2 MB on chrome process.
(In reply to Benoit Girard (:BenWa) from comment #7)
> The scrollbars I see in gaia are done in JS, are you sure they need XUL?

AFAIK Gaia has nothing to do with scrollbar.
(In reply to Alive Kuo [:alive][NEEDINFO] from comment #11)
> (In reply to Benoit Girard (:BenWa) from comment #7)
> > The scrollbars I see in gaia are done in JS, are you sure they need XUL?
> 
> AFAIK Gaia has nothing to do with scrollbar.

Right that was a mistake.
(In reply to Kai-Zhen Li from comment #10)
> From Bug 894927 comment 26, I think we can save at least 1~2 MB on chrome
> process.

Why do you believe that that comment is relevant to this?
Also, even if it is, decreasing VSS is not particularly useful.  Decreasing RSS is what matters.  The phone is limited by physical memory, not virtual address space, so if binary code gets loaded into VSS and discarded because it's not used it doesn't affect us much.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #14)
> Also, even if it is, decreasing VSS is not particularly useful.  Decreasing
> RSS is what matters.  The phone is limited by physical memory, not virtual
> address space, so if binary code gets loaded into VSS and discarded because
> it's not used it doesn't affect us much.

Kyle, Thanks! Yes, we should focus on RSS.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #14)
> Also, even if it is, decreasing VSS is not particularly useful.  Decreasing
> RSS is what matters.  The phone is limited by physical memory, not virtual
> address space, so if binary code gets loaded into VSS and discarded because
> it's not used it doesn't affect us much.

From my understanding when you touch a code page it never gets paged out on mobile. So if we execute a large code surface of xul to provide the features mentioned above we're paying the cost permanently. Right?
(In reply to Benoit Girard (:BenWa) from comment #16)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #14)
> > Also, even if it is, decreasing VSS is not particularly useful.  Decreasing
> > RSS is what matters.  The phone is limited by physical memory, not virtual
> > address space, so if binary code gets loaded into VSS and discarded because
> > it's not used it doesn't affect us much.
> 
> From my understanding when you touch a code page it never gets paged out on
> mobile. So if we execute a large code surface of xul to provide the features
> mentioned above we're paying the cost permanently. Right?

If that's true it would be very unfortunate.
(In reply to comment #17)
> (In reply to Benoit Girard (:BenWa) from comment #16)
> > (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #14)
> > > Also, even if it is, decreasing VSS is not particularly useful.  Decreasing
> > > RSS is what matters.  The phone is limited by physical memory, not virtual
> > > address space, so if binary code gets loaded into VSS and discarded because
> > > it's not used it doesn't affect us much.
> > 
> > From my understanding when you touch a code page it never gets paged out on
> > mobile. So if we execute a large code surface of xul to provide the features
> > mentioned above we're paying the cost permanently. Right?
> 
> If that's true it would be very unfortunate.

If we're using glandium's on the fly decompressor linker, then that would be true, since we don't have swap space to back those pages.  But also note that currently the XUL binary code should be fairly hot given the scrollbar implementation.
Attached patch no-xul.patch — — Splinter Review 
That's my current wip rebased. It's not actually removing *all* of xul, for instance everything from under /dom/interfaces/xul is still there.

I had to move some the nsIController(s) and the nsXULController implementation - they are actually not bound to any xul markup, and not building them breaks all the input fields.

I don't have yet memory comparison numbers available.
Attached patch less-xul.patch — — Splinter Review 
This patch make us not build dom/interfaces/xul
I am implementing scrollbars in the compositor. We won't need any of the widget/xul code.
blocking-b2g: --- → 1.3?
Whiteboard: [tarako]
triage: remove dependency for tarako project as this should not have obvious gains for tarako
No longer blocks: 128RAM
blocking-b2g: 1.3? → ---
Whiteboard: [tarako]
(given that this is assigned, assuming the right person for this has already been found)
Flags: needinfo?(mwu)
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: