106161 - merge Content & Layout libraries

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Cathleen]

intend to shave off some footprint.
hyatt claims doing so, he can shave off 8 bytes per XBL binding, we've got
thousands of bindings.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Just to reiterate what I said in another other bug: I oppose combining these two
libraries because the reasons they were split in the first place have not
changed. For example decoupling things, preparing for server side DOM module,
compile time halved for content-only/layout-only changes. Moving style out of
content would be ok by me, though.

Comment 2

[Chris Waterson]

First of all, building a server-embeddable DOM is a non-goal. And even if it
were a goal, we've driven so far off into the weeds that getting back on the
road with our current codebase would be a major challenge. Why would a server
DOM care about mapping stylistic impact of attribute changes? (Disentangling
this would be a fair amount of work.) Why would a server DOM care about all of
the cruft we do for incremental content construction? Why would a server DOM
care about the residual style fixup that happens in the parser? Why would a
server want to import all of our component management and scripting mess when it
probably already has mechanism for this?

Whew. But if you _still_ want to do this, then go ahead: create a ``server DOM''
library that links only the necessary static libs for the content model. The
changes proposed here certainly do not prevent us from doing that.

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

We discussed this a bit during lunch, so I'll just refresh some of what was
said. The server-side DOM would include MSXML-like (MozXML?) package that you
could load into any native application (server or not), parse XML and get DOM
access to it.

The XML side of things is pretty clean (of layout) compared to HTML, but maybe
the DOM mixes hopelessly with layout. I don't know if there is any interest in
other than XML-module like this out there.

Looking _only_ at content/xml, it would seem MozXML would not be too hard to do
the way you described. 

But my fear is, as I have stated before, that combining content and layout means
it becomes very easy to mix these two areas so badly that they can "never" be
separated. It can also mean it may become increasinly hard to fix content-only
or layout-only bugs. Just look at the form control stuff that is now being moved
from frames to content. It is a useful separation, also helping engineers focus
their work in their strong areas.

Anyway, I think dp took the task of doing the merging by making content a static
library that is linked with layout. That may help maintain the logical
separation, while getting the perf/footprint benefits at the same time. Of
course that will still make building content/layout more tedious and time consuming.

Comment 4

[Suresh Duddi (gone)]

Right. Plan is
- content linked statically into layout
- content seizes to be a module in itself anymore

Comment 5

[Chris McAfee]

"ceases", but we all got it :-)

Comment 6

[Axel Hecht]

would it make sense to have the xml-content have it's own MODULE, so the
REQUIRES would ring a bell once someone ties to strongly between the stuff?

Comment 7

[Suresh Duddi (gone)]

Ccing seawood.

Comment 8

[Suresh Duddi (gone)]

Attachment: Proof of concept works on windows. Comments welcome.

Comment 9

[Suresh Duddi (gone)]

Attachment: layout/build/nsLayoutMetaModule.cpp - Part of previous patch

Comment 10

[Peter Van der Beken [:peterv]]

So now we'll have to build two libraries every time we make a change in content
and I do remember the painful link stage we had before the split. Bah!
To me content and layout are seperate areas, even though they're closely
related. I have made changes in content, but I don't remember ever touching
layout after the split. This merge will blur the line between them even more. I
second Heikki's objections.

Comment 11

[Chris Waterson]

peterv: Making life easier for a developer (i.e., _you_) to build content is
trumped by performance and footprint improvements. Sorry, but that's life. It's
time for our Big Fat Baby to go on a diet and removing bogus layers of
abstraction is part of the Weight Watchers plan.

So, rather than whining about how this makes it hard for you to build, help dp
figure out a way that he can satisfy your desire to build a standalone content
library. dbaron had some ideas in this regard; for example, he suggested we
might maintain the current layout/content split if we could:

1. Move style back to layout and remove the attribute mapping logic from
   the content elements.

2. Determine if it is possible to move all of XBL to layout, and
   use opaque pointers in content for stuff that requires the binding manager.

Comment 12

[Suresh Duddi (gone)]

REMINDER: Got to update any packaging files and 
http://lxr.mozilla.org/seamonkey/find?string=basebrowser about the missing
gkcontent.dll when we land. - thanks jud 

Comment 13

[hacker formerly known as [e-mail]]

This bug just leaves a nasty taste in my mouth all around.  

First, the merging of these two libraries is going to quite painful for all of
us not compiling on 1.5 Ghz boxes.  What part of deCOMtamination requires
merging the libraries?  Why can't layout & content be further broken up into
sub-libraries that are used by the main layout.dll instead?  In my
sleep-deprived state, I'm envisioning separate dlls for html/forms, tables, xul,
etc. In short, all of the things that we may not want in an embedding build.

Second, why bother to build content as a static lib and then link its entire
contents into layout?  That's a waste of time & diskspace.  It's bad enough that
we do it for the existing sublibs of content & layout.  Those intermediate libs
need to go away completely. (Insert Shaver chanting "RMCH! RMCH!")  Plus I'm not
keen on branching out the use of -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT which we
concocted specifically for the static builds.

Comment 14

[Chris Waterson]

Okay, I confess: ``merging content and layout'' is a solution. Let's talk about
the problem and work from there.

We want to:

1. Avoid the overhead of virtual dispatch where we can

2. Avoid reference counting where ownership can be direct

3. Improve call signatures to take advantage of register return instead of
   forcing stack memory references; i.e.,
   
      nsFrame* GetNextSibling() const

   instead of 

      NS_IMETHOD GetNextSibling(nsIFrame** aResult) const

We specifically want to do these things on objects that we _know_ are incurring
penalty. In the case of (1) and (3), because vtune has shown us a speed win; in
the case of (2), because trace-malloc has shown us (that saving a word for the
refcnt is) a space win.

Because of linkage issues between component DLLs, there are severe constraints
on  our ability to do (1), and at this point (1) is what we have the most data
on (nsIStyleContext::GetStyleData, e.g.). In theory (2) and (3) could be done
without changing any linkage issues.

The objects we'd like to perform this surgery on include:

- style contexts, where we'd like to eliminate nsIStyleContext and just use the
  style context object directly, hopefully benefitting from (1) and (3). (I
  believe that style contexts must continue to be refcounted due to the fact that
  they may be shared between frames.)

- frames, where we'd like to eliminate nsIFrame and just use the nsFrame object
  directly. In this case, we'd see benefits from (1) and (3), because
  frames aren't refcounted anyway.

- some of the XBL object interfaces (hyatt, help me out here), which purportedly
  could benefit from all three optimizations.

So...as I'd mentioned, dbaron suspects it may be possible to reap the rewards of
(1), (2), and (3) on objects that matter without re-merging layout and content
(reshuffling files may be required, of course). Merging layout and content
wholesale is simply use of brute force to accomplish the above. If people are
willing to think more carefully about how to do this in a surgical manner, glory
be. Send your patches to dp.

Comment 15

[David Hyatt]

I inspected XBL a bit more closely last night, and it appears that a merging of
content and layout is not required to decomify XBL.  With some minor changes to
interfaces, I can decomify nsIXBLBinding and only refer directly to bindings
from the content DLL.  If we refactor style, we can also avoid having to merge
the DLLs.  Maybe we should examine all of the other classes (besides CSS/XBL) a
bit more closely before rushing into a merge.

Comment 16

[Suresh Duddi (gone)]

Alright. nsIStyleContext being the only other optimization that could warrant
merging the dlls, I will try to reshuffle it to see if we can move style out of
content/ and into layout/ We will talk again then.

Comment 17

[Suresh Duddi (gone)]

Back to dbaron - he has the best handle on the nsIStyleContext becomming a
non-COM object.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Giving this back to dp, since I think this will really need to be done if we're
really going to deCOMify nsIStyleContext in the near future.  Sorry.

Comment 19

[Johnny Stenback (:jst)]

Can't we pull off something similar to what was done for nsIFrame and
nsINodeInfo with nsIStyleContext too?

Doing this would make working on content and layout code suck so bad that IMO we
need a really good reason to do this.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

> Can't we pull off something similar to what was done for nsIFrame and
> nsINodeInfo with nsIStyleContext too?

How would you suggest doing that?  GetStyleData involves a good bit of code, so
we can't really inline it.  (We could do it for GetParent, but GetStyleData is
probably the bigger win.)

Comment 21

[Johnny Stenback (:jst)]

Don't know that code well enough to say. Maybe we could deCOMtaminate the code
and move it into the shared (or static, rather) library we already have that's
used by both content and layout?

Comment 22

[Suresh Duddi (gone)]

We really need to make a decision on whether we should do this for 1.0

Comment 23

[Suresh Duddi (gone)]

I doubt we could get to this for mozilla1.0 Removing all nominations. Moving to 1.2

Comment 24

[Brian Ryner (not reading)]

I'll take this, dp is gone.

Comment 25

[Brian Ryner (not reading)]

So, along the lines of what jst said, it looks like it would be possible to move
nsStyleContext into content/shared and avoid the virtual dispatch cost for that
object from both the content and layout libraries.  However, there would be code
bloat (duplication) that we'd need to consider.  We could also look at making
content/shared into a shared library, but I think we'd then incur indirect call
(cross-library) overhead that's comparable to the virtual function call overhead
we're trying to get rid of.  nsStyleContext.cpp isn't a particuarly large piece
of code, so maybe moving it to content/shared is the way to go.

Still, I think merging content and layout (and maybe other libraries, too) would
be a Good Thing. The division between content and layout is useful for us as
developers, but it's silly to let it dictate a code design that ultimately
results in slower performance.  If anyone wants a standalone 'content' xpcom
component, a build-time option seems an appropriate way to deal with that.  I
also think it's likely that the average developer system is now beefy enough to
handle linking a debug version of the combined library.  That would let us get
rid of _so_ many interfaces (remember, it costs us 4 bytes per object per
interface) and just pass around concrete pointers within gecko, and get rid of
the COM function signatures which not only hurt performance (as waterson
mentions) but also, in my opinion, dramatically hurt readability and
maintainability of the code.

Comment 26

[Alec Flett]

Since I've been doing some DLL/bloat work, I thought I'd voice some things to
think about here: 
- I like the idea of having a combined dll, even though it could still suck for
developers. Part of me wanted to say "can't  
- Do we have an idea of the size of the content/shared stuff, as it sits in each
dll? How much savings will we get from "removing" the rundant code?
- Is there a list of structures/interfaces that we could remove the vtables
from? Do we know how often these objects are created? If we had such a list
(even a rough list) then we could measure the runtime performance benefits too,
and file bugs to make sure they actually get fixed. 
- We could also use the list to explore arena allocation of some of them too.
(since its so much easier to arena allocate objects that aren't XPCOM objects)

Comment 27

[Brian Ryner (not reading)]

Attachment: patch

I'd sort of like to do it this way, without the metamodule stuff (this avoids
creating an intermediate content static library)

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Seems fine with me.

Comment 29

[Alec Flett]

I like the general approach as well. I'm assuming that most of those files are
just making their way over from content/build.. any chance that you can get
someone with cvs access to actually copy the cvs ,v versions over directly
inside the repository, so we have revision history?

Comment 30

[Brian Ryner (not reading)]

Per conversation with jst, we probably want to take this opportunity to
reorganize the directory structure of content and layout.  We had discussed just
making the current content and layout directories be parallel under layout/, i.e.:

layout
|- build
|- content (current content/)
|  |- base
|  |- events
|  |- ...
|- frames (or presentation? - current layout directory)
   |- base
   |- html
   |- mathml
   |- xul

This could likely use some further refinement.

Comment 31

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I had a proposal for how to reorganize layout in another bug somewhere.

Comment 32

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The bug I was thinking of was bug 107101, which was just about CSS.  But there
are other changes I'd want to make, especially moving the document viewer, the
printing stuff, and the docloader factory back from content to layout.  Also,
the XUL stuff that's inside of layout/xul/base/src/ should be unnested (subdirs
of layout/xul/), and the layout/base/src and layout/html/base/src/ distinction
should be cleaned up.

With the structure above, perhaps events and xbl belong at toplevel rather than
within content?

Comment 33

[Brian Ryner (not reading)]

see bug 187719 for one idea that might offset the increase in memory required to
link the library (at least on unix, where iirc the problem is the worst).

Comment 34

[Alec Flett]

so bryner and I were talking about this yesterday - what do we have to do to get
moving on this? it sounds like we need to agree on a new hierarchy?

I thought a little more about the whole file hierarchy thing and I almost wonder
if it is really worth it to move/copy files around given the headache its going
to cause for people who need to do things like back-port fixes to previous
branches, merge changes in from branches that have recently branched, and so forth. 

I'll leave it up to you guys but basically I'd really like to see this happen
sooner rather than later, especially given the benefits to the deCOMification
efforts and I'd be really disappointed to see this miss 1.3 because of an
indecision..

Comment 35

[Boris Zbarsky [:bzbarsky]]

> if it is really worth it to move/copy files around 

The current setup is illogical and nearly unusable.  I often find myself looking
in 2 or 3 places before finding the file I need and I _know_ where they live,
for the most part... crap like headers in layout/ while the .cpp is in content/
don't help any.

My take on the filesystem relocation is that it too should happen sooner rather
than later...

Comment 36

[Alec Flett]

I should also add that at some point I'd like to merge the GFX and Widget DLLs,
and in that process will probably also incorporate the view dll. I'd also like
to merge the parser DLL into the DLL we describe here as well.

I've got a whole new idea: perhaps we can just make a new toplevel directory,
"gecko", and build the final DLLs there? I can imagine:

gecko/base would contain {content, layout, parser, more? }
gecko/widget would contain { gfx, widget, view, maybe plugin? }

all the other directories would just make static libraries and it would be up to
 the gecko/ toplevel to figure out how to glue them together.   

Lets face it - we should not be using DLLs as a means to organize our code, and
we don't need this many DLLs. We should only depend on DLLs for code
organization when granularity of features can be implemented in such a way that
you can actually remove a DLL to lose a feature, and still have the rest of the
product run fine.

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Last I heard, "Gecko" is a Netscape trademark and should not be used in Mozilla.

Comment 38

[Benjamin Smedberg]

I like the idea of a new directory for the entire layout structure. How about
nglayout/

Comment 39

[Mike Shaver (:shaver emeritus)]

"Netscape Gecko" is a trademark, I believe, but "Gecko" isn't (I remember
discussions with Eric Krock on this topic, many moons ago).  Of course "Mozilla"
is a Netscape trademark too, so here we are.

Comment 40

[Brendan Eich [:brendan]]

Please don't start mooting (over-deep) source tree straw-men.  What we need are
some guiding principles that everyone can agree to.

Source trees should be flat, with modules (whether they have frozen APIs or not)
crowding a top-level, because we do not know what "embedding" or "application"
combinations will be important in a year or three.  The problem we have is not
lack of tyranny-of-hierarchy, it's too many silly content/layout and public/src
(sub-)directories.  Oh, and we do have tyranny of hierarchy too, with the deep
sub-tree at embedding.

What's more, you're not gonna move mozilla/js under mozilla/gecko.  So let's
table the gecko useless top-level sub-directory, if we can't agree to drop it.

If you want to clean things up, you also need to find out what projects and
companies have active branches (Sun China?), and get them to agree to the reorg.

The reorg should follow some principles that can be applied to other reorgs and
to new code.  The old public/src/idl separation was over-used, along with way
too much XPCOM.  What alecf said in the final paragraph of comment #36 (about
not bending source tree layout around DLL or DSO organization) is dead on, but
how does that translate into a procedure we can use in this reorg?

More in a bit, feel free to add ideas (is bugzilla threaded yet?  Oh well, there
is no stopping this discussion; I guess moving it to m.layout is unlikely, too).

/be

Comment 41

[Brendan Eich [:brendan]]

Bryner wanted me on the record saying that comment #25 and immediately after
point the way in the short run.  Comment #30 and beyond, on source tree reorgs,
do not pay for themselves yet.  Let's defer them until developer pain and better
ideas (principles of operation, concrete tree reorg plans) evolve to the point
where it makes sense to sink the big cost of a tree reorg.

Right now, we want deCOMtamination and reduction of inter-DLL calling overhead.

/be

Comment 42

[Alec Flett]

I should clarify my "gecko" toplevel directory.. I am NOT proposing that we move
any files under gecko/

my proposal is that this toplevel directory is the master directory for creating
the DLLs that are otherwise constructed beneath the toplevel.

An example:
gecko/base might build gecko.dll, and be composed of:
* gkbase_s.lib from layout/base/src
* gkhtmlforms_s.lib from layout/html/base/src
* gkview_s.lib from view/src
* gkconbase_s.lib from content/base/src

the point is that the modules themselves are only responsible for creating
static libraries, and that the actual formation of the DLLs happens
independently in another directory.
I can also imagine that there might be a gecko/lite directory that builds
gklite.dll from a subset of these dlls (dropping XUL support, as an example)

So you see, the actual packaging up of the code into a DLL happens independent
of the module itself - a module is not a DLL. a module is a logical grouping of
code.

so that the gecko directory might look like:

gecko
  +--base
  |   +--nsGeckoBaseModule.cpp
  |      Makefile.in
  +--widget
      +--nsGeckoWidgetModule.cpp
         Makefile.in
     Makefile.in

and that's IT - no more subdirectories below "base" or "widget"

Comment 43

[Brendan Eich [:brendan]]

alecf, thanks for clarifying -- I was thrown by your listing of other subdirs in
comment #36, and then others seemed enthusiastic about wholesale source
relocation under a bogo-top-level.

Would your proposal help people who complain that deCOMtaminating without
unifying content and layout requires a build in both, even if you change only
sources under content?  I.e., would gecko/Makefile.in be smart enough to kick of
necessary makes in peer (top-level under mozilla/) directories?  Should it be so
smart?

/be

Comment 44

[Alec Flett]

I wasn't thinking it would be that smart - though I feel like the whole "being
able to rebuild layout from one toplevel dir" is a bit of a pipe dream.. at
least based on my own personal habits.. if for instance I was to change
layout/html/forms/src/nsHTMLButtonControlFrame.cpp, I would usually just do a
make from layout/html/forms/src and then layout/build - with the toplevel gecko,
I'd have to do layout/html/forms/src and gecko/ so the situation wouldn't change
much..I feel like you're wasting your time to wait for an entire build sweep
through layout/ when you only want to build one or two directories.

I have this trick I sometimes use, that maybe we could incorporate into the
build system:

echo layout/html/forms/src layout/build | xargs -n1 make libs -C

I like the idea of being able to say

make FORCEDIRS="layout/html/forms/src layout/build"

but I haven't done the work to see how hard that would be.

Anyway, I'm digressing. The short answer is "no, but we would be no worse than
we are now"

Comment 45

[hacker formerly known as [e-mail]]

FWIW, I'm working on the toplevel-rebuild pipe dream in the form of bug 167254.
 I'm not so keen on the 'create more static libs to link into a shared' lib idea
though.  On MSVC, linking the static libs into a shared lib seems to be a no-op
but it seems to take some time for both the static lib creation and then the
shared lib creation when using gcc.  We've been trying to come up with ways to
get rid of those static libs for awhile (as far back as bug 11219).  They also
cause problems with the dbx debugger on solaris (bug 146154). 

Comment 46

[Brian Ryner (not reading)]

Comment on attachment 110550 [details] [diff] [review]
patch

I'll post a new version of this that includes installer package changes and Mac
CFM build changes.

Comment 47

[Kevin McCluskey (gone)]

"gecko/base would contain {content, layout, parser, more? }
gecko/widget would contain { gfx, widget, view, maybe plugin? }"

It is would be better to combine view with layout not "gfx/widget". The view
system is cross-platform and closely tied to layout. Layout is responsible for
creating and positioning views etc. The view system paints by making calls to
layout frames.

gfx/widget contains 90% platform specific code. It is an abstract toolkit mapped
to native toolkit calls.  It is not tied to the view system and could be used
independently of the view system. 

Comment 48

[Kevin McCluskey (gone)]

"gecko/base would contain {content, layout, parser, more? }
gecko/widget would contain { gfx, widget, view, maybe plugin? }"

It is would be better to combine view with layout not "gfx/widget". The view
system is cross-platform and closely tied to layout. Layout is responsible for
creating and positioning views etc. The view system paints by making calls to
layout frames.

gfx/widget contains 90% platform specific code. It is an abstract toolkit mapped
to native toolkit calls.  It is not tied to the view system and could be used
independently of the view system. 

Comment 49

[Brian Ryner (not reading)]

Attachment: new patch

Same idea as the previous patch, i.e. this does not involve a directory
structure reorganization.  This one includes Mac CFM build changes and
xpinstall package changes.

Also, see bugscape bug 21931 for xpinstall package changes to the ns tree.

Comment 50

[Brian Ryner (not reading)]

Comment on attachment 111264 [details] [diff] [review]
new patch

(note that if this patch gets reviewed, I'll get files moved from content/build
to layout/build in the repository)

Comment 51

[Brian Ryner (not reading)]

Attachment: fixed a couple of build problems

Changes from the previous patch:

- included change to MozillaBuildList.pm for Mac CFM builds
- export nsContentCID.h from content/base/public instead of layout/build, so
that it still ends up under dist/include/content.  This lets us avoid changing
some REQUIRES (which helps keep content and layout logically separate)

Comment 52

[Brian Ryner (not reading)]

Comment on attachment 111314 [details] [diff] [review]
fixed a couple of build problems

carrying over dbaron's r= (I already talked with him about the nsContentCID.h
change).

Comment 53

[Johnny Stenback (:jst)]

Comment on attachment 111314 [details] [diff] [review]
fixed a couple of build problems

sr=jst

Comment 54

[Brian Ryner (not reading)]

checked in.  there was a drop of just over 100KB in static footprint.

Comment 55

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

since this landed (which i still think was a bad idea), i wanna make a plea to
everybody to still keep logic separated.

This bug going in is *not* an excuse to mix up layout code in the elements and
to make layout use private members in the document-object. The stylesystem, the
content model and the layout code are separate things, not one big happy mess.

The reason for this bug was deCOMtamination, nothing else.

Comment 56

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This is of course not only the responsibility of the people that makes patches,
but also of the reviewers and superreviewers

Comment 57

[alanjstr]

Oh bugger, I didn't mean to do that.