The layout library is too big...it needs to be split up

VERIFIED FIXED in mozilla0.9

Status

()

defect
P3
normal
VERIFIED FIXED
19 years ago
19 years ago

People

(Reporter: cls, Assigned: hjtoi-bugzilla)

Tracking

(Depends on 1 bug)

Trunk
mozilla0.9
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(20 attachments)

3.51 KB, text/plain
Details
1.99 KB, text/plain
Details
86.59 KB, patch
Details | Diff | Splinter Review
99.25 KB, patch
Details | Diff | Splinter Review
2.06 KB, patch
Details | Diff | Splinter Review
1.81 KB, patch
Details | Diff | Splinter Review
9.98 KB, text/plain
Details
172.50 KB, patch
Details | Diff | Splinter Review
134.35 KB, patch
Details | Diff | Splinter Review
2.13 KB, patch
Details | Diff | Splinter Review
10.64 KB, text/plain
Details
172.59 KB, patch
Details | Diff | Splinter Review
139.07 KB, patch
Details | Diff | Splinter Review
115.14 KB, patch
Details | Diff | Splinter Review
135.49 KB, patch
Details | Diff | Splinter Review
5.43 KB, patch
Details | Diff | Splinter Review
11.04 KB, text/plain
Details
5.57 KB, patch
Details | Diff | Splinter Review
197.58 KB, patch
Details | Diff | Splinter Review
153.36 KB, patch
Details | Diff | Splinter Review
Reporter

Description

19 years ago
libraptorhtml has always been huge but recently there was a noticeable change in
the amount of resources needed to compile libraptorhtml.  I can no longer build
mozilla without freeing up resources by killing existing processes (namely
netscape) first.  Given that the acheron tinderbox started failing to link
smaller libraries during the recent string changes, I suspect the string changes
but I have no real evidence.

Along the same vein of splitting up libraptorhtml, I'm interested in what's
required to make certain layout features optional.  The directories under
layout/ appear to be functionally segregated but would it really be that easy to
separate out xml/xbl/xul from the core html code?  Or tables & forms from css?

Comment 1

19 years ago
waterson, I'll let you known this one, I hope that is ok.
Assignee: clayton → waterson

Comment 2

19 years ago
woof
Status: NEW → ASSIGNED

Comment 3

19 years ago
I have 128mb real memory and 256mb of swap on my new redhat 6.2 machine.
When I try to compile mozilla I always crash linking one of the libraptor
libraries. ld crashes, allegedly because the library is too big. Maybe
if i tried killing the browser first.

ha ha! the hardware requirements in the build instructions say "32 MB RAM, 
128 MB swap (64 MB RAM recommended)" Is there any chance of fixing things so
that's true again or should I update the docs? (4 GB ram recommended)


http://www.mozilla.org/build/unix-details.html
Reporter

Comment 4

19 years ago
I think I know how to split up layout into separate modules (based upon info in
nsLayoutModule.cpp) but I'm still wondering where the split should occur.  Right
now, it takes about 110 megs to link libgklayout.so on my machine.  If I just
link in the static libs from the layout/html subdirs, it still takes 80megs to
link the library.  If I remove forms, tables, & style from that list, it knocks
it down to about 60 megs.  And of course, those numbers are w/o the necessary
code fixes needed to remove any interdependencies.

My build machine is a dual PII-333 with 128MB ram + 128Mb swap.
The current structure of layout doesn't make much sense to me.  My rather 
uninformed opinion is that the clearest division seems to be between content 
model and display model, although there are multiple copies of each...

mixed:
  base/
  html/style/ - SS "content" model; creates display model (CSSFrameConstructor)

Content model / DOM related:
  html/content/
  html/document/
  events/
  mathml/content/
  svg/content/
  xbl/ (I think)
  xml/
  xsl/

display related:
  html/base/
  html/table/
  html/forms/
  mathml/base/
  svg/base/
  xul/

I would think future modularity in layout might want to make a distinction like 
this:  there is a content model/DOM code (which can be used on its own - XML as 
data), a style system that either converts a content model either to a display 
model (CSS) converts it to another content model (XSLT), or handles special 
content elements that have a display model associated with them (HTML Form 
elements, MathML, SVG, XUL, XSL-FO), and then various display systems to 
actually display stuff.  I'm not sure how one could elegantly handle all these 
possibilities, but I think any split of layout should at least consider these 
issues so we move in the right direction.

How were others thinking of splitting layout?

Comment 6

19 years ago
-> mfuture, unless this becomes an issue for embedding. jud?
Target Milestone: --- → Future

Comment 7

19 years ago
making certian pieces option would be nice. ideally this would happen in the
build config though, we'd like to minimize the number of actual shared libraries
that we distribute.
Reporter

Comment 8

19 years ago
*** Bug 63666 has been marked as a duplicate of this bug. ***

Comment 10

19 years ago
Cc:ing people in the XML/DOM group since they've been talking about trying to
split out content from layout with the goal of eventually being able to deliver
a parser/DOM component for use outside the browser.
Just to comment that I am working on this as well...

I have moved the stuff David mentioned (+/- a little) in the attachment to a new
toplevel content directory. I copied nsHTMLAtoms and nsXULAtoms as well (there
are a few unneeded nsXULAtoms includes but nsDocument makes some real use of
them). I can currently compile base, build (haven't really looked at this, and
fails eventually 'cos html not compiled), events, xbl, xml, xsl and xul. html
dir is the last one where I actually need to do some work, creating new
interfaces etc. After I can completely build the new content I'll see what I can
do to layout, and then fix the makefiles properly (I am doing this on Windows
right now,... need to change the library names as well...)
Are you doing this by exporting files that shouldn't be exported or by breaking
the dependencies I mentioned?
At the moment I am not exporting anything more than we were exporting (except
for new interfaces). I have copied the atoms, so right now they are both in
layout & content (and rdf - XUL atoms). If it works this way, it is of course
bad for maintenance and should be fixed somehow. But the atoms still worry me,
'cos I can't see how they would work if we have copies of atoms in different
DLLs (for example, nsHTMLAtoms::id gets different value in each DLL and if that
value gets passed between DLL boundaries atom (pointer) comparisons are
screwed???). If it should just work, somebody needs to straighten my head...
I think it should just work, because they're all created by NS_NewAtom.  I
guess the main reason for nsLayoutAtoms, nsHTMLAtoms, etc., is to avoid
refcounting the atoms all the time.
Doh, that was it, thanks David. jst said atoms are nowadays made in xpcom (I
looked but didn't see ;) but I didn't look where NS_NewAtom was implemented... 
I have the new content directory building and linking except for 1 unresolved
external. I'll look at building the remaining layout now. I'll attach the linker
errors if anyone is interested in finding a solution to it at this point.
Target Milestone: Future → mozilla0.8

Comment 18

19 years ago
nsUniqueStyleItems 
seems to be a struct 
member (http://lxr.mozilla.org/seamonkey/ident?i=nsUniqueStyleItems) and it 
resolves to the core raptor lib (layout/base/src). You'd either have to pull it 
(or the structs that use it) out of your lib, link against the main layout lib 
(not advisable statically :-)), or pull it out of the main layout lib and link 
it into yours.
I'll attach some snapshot diffs of where I am right now. I must say I botched
things somewhat by wiping out my CVS dirs in the new content directory so the
diff in there is not totally reliable at the moment. I have 2 unresolved
externals while building the new content, and 5 unresolved externals while
building the remaining layout.

I am not building tests, nor mathml nor svg at the moment.

I know some of the changes I have made are wrong (trying to get past compiler
errors fast and then see how the linker does etc.).

There is some duplication of files which is not necessary, and some duplication
which is.

I'll continue on Tuesday...
All but one of the link errors you get when linking layout should be easy to fix
by using something like this:

  nsCOMPtr<nsIElementFactory>
ef(do_CreateInstance(NS_ELEMENT_FACTORY_CONTRACTID_PREFIX "text/html");

  ef->CreateInstanceByTag(...);

The one that can't be solved with that is the anonymous element creation, for
that you need some other solution.
It flies... well, getting there... Everything compiles and links on Windows
(hrmph, now I have a totally bizarre linker error in xpinstall for unresolved
nsRect::nsRect(void)). When I try to run, I get loads of bad asserts and then
crash (which is what I expected since I need to work on the build directories...).

I'll attach the final (?) makecontent.sh and the current diffs. The changes
outside layout/content are minimal, just including the new nsContentCID.h. I
needed to export a little more stuff, and I resolved the nsUniqueStyleItems
unresolved external by moving nsStyleSet, nsHTMLStyleSheet and nsStyleContext to
content as well, which is not too nice...
It works! This message written with content DLL separated from layout. I see a
few oddities, like text fields being weird lengths. And there is still the
xpinstall DLL which won't link. I'll attach the latest diffs.
Severity: normal → blocker
Component: Layout → ActiveX Wrapper
Priority: P3 → --
Hardware: PC → All
Target Milestone: mozilla0.8 → M1
Another oddity is that you're setting all select elements to the first choice.
Restoring state of bug.
Severity: blocker → normal
Component: ActiveX Wrapper → Layout
Priority: -- → P3
Target Milestone: M1 → mozilla0.8
Doh! Thanks David. Hmm... I also see file controls not working. But all in all
it looks pretty good now.
I've solved most of the problems. Things that I still know of: My Windows build
from early this week crashes on shutdown and I get an assert viewing a file
through the FTP protocol. On Linux, build from yesterday (yes, it now runs there
as well) it seems some element placements are not correct and returning back to
an FTP page it shows up blank. I also got some autocopy+mouse cursor+JS errors
after I did drag&drop. Precheckin tests pass otherwise, except for mail which I
haven't been able to use with Mozilla anyway in a long time.

It also compiles with MOZ_SVG and MOZ_MATHML but there are linker errors (11
unresolved externals, but they should be easy to fix - inline some functions and
replace NS_New... with do_CreateInstance).

I don't have a Mac, so it would be great if somebody would be able to try this
there and create/modify the Mac makefiles.

Some tips on applying patches: it seems the root level makefiles confuse patch,
so what I did was to temporarily rename the root level makefiles and remove them
from the diffs, and after applying the big patches apply the makefile patches
individually.

I'll attach new diffs.

Comment 41

19 years ago
Heikki, this bug is all yours, baby! :-)
Assignee: waterson → heikki
Status: ASSIGNED → NEW
Ok, I do not see any glitches that I think are caused by these changes.

I still can't link xpinstall/src, and I have not found what is causing this
breakage. I think it is real, though, because today I pulled a fresh tree before
tree opening and applied the changes in the latest diffs. This is what the
Windows linker is saying:

+++ make: install in d:\content2\mozilla\xpinstall\src
        echo +++ make: Creating DLL: .\WIN32_D.OBJ\xpinstal.dll
+++ make: Creating DLL: .\WIN32_D.OBJ\xpinstal.dll
        link @WIN32_D.OBJ\cmd.cfg
   Creating library .\WIN32_D.OBJ\xpinstal.lib and object .\WIN32_D.OBJ\xpinstal
.exp
jsdombase_s.lib(nsScreen.obj) : error LNK2001: unresolved external symbol "__dec
lspec(dllimport) public: __thiscall nsRect::nsRect(void)" (__imp_??0nsRect@@QAE@
XZ)
.\WIN32_D.OBJ\xpinstal.dll : fatal error LNK1120: 1 unresolved externals
NMAKE : fatal error U1077: 'link' : return code '0x460'
Stop.
NMAKE : fatal error U1077: 'd:\PROGRA~1\MICROS~1\VC98\BIN\nmake.exe' : return co
de '0x2'
Stop.

nsScreen.cpp is using nsRect (default constructor), which is inlined function. I
have not touched nsScreen.cpp or anything in gfx or xpinstall. I tried including
nsRect.h explicitly in nsScreen.cpp and addling gfxwinbase.lib to be statically
linked with the xpinstall DLL but this does not help (as it shouldn't). I also
tried changing one of the default constructor calls in nsScreen.cpp to
nsRect(0,0,0,0) and this resulted in the linker compiling about that as well
while building xpinstall. Including nsRect.h and declaring an nsRect variable in
one of the files in xpinstall only resulted in a second instance of the same
unresolved external.

The weird thing is that there are other places where jsdombase_s.lib in linked
but they work fine.
Reporter

Comment 46

19 years ago
Recommending pushing this off to 0.9 since 0.8 is a week away and I doubt this
has been heavily tested by anyone outside of the cc: list.
Keywords: mozilla0.9

Updated

19 years ago
Blocks: 43121
I will not be able to check this in for 0.8, moving to 0.9.
Target Milestone: mozilla0.8 → mozilla0.9
I'll look at porting this to Mac.
I got a diff from jst to xpinstall which solves the Windows linker error there
(I tested, thanks Johnny). So Peter, if you could have your stuff done by early
next week it would be perfect.

The patches here should work as is, but nsGenericHTMLElement.cpp needs a little
new work. Where it uses nsObjectFrame you need to change the include to
nsIObjectFrame and copy the code that gets plugin instance from
nsHTMLEmbedElement.cpp, something like:

     if (type.get() == nsLayoutAtoms::objectFrame) {
-      // XXX We could have created an interface for this, but Troy
-      // preferred the ugliness of a static cast to the weight of
-      // a new interface.
-
-      nsObjectFrame* objectFrame = NS_STATIC_CAST(nsObjectFrame*, frame);
+      nsIObjectFrame* objectFrame;
+      result =
frame->QueryInterface(NS_GET_IID(nsIObjectFrame),(void**)&objectFrame);
+      if (NS_FAILED(result)) {
+        NS_ABORT_IF_FALSE(0,"Should not happen - frame is not object frame even
though type is nsLayoutAtoms::objectFrame");
+        return result;
+      }
Status: NEW → ASSIGNED

Comment 50

19 years ago
Good job. Compilation is great at this end (where numerous compilations are
needed to try to get the single pixel at its right place...)
Ok, since we landed this yesterday I am going to mark this fixed.

We had a few hair rising incidents while checking, like Peter (who was doing the
Mac side of things) losing network connection just when we were checking in and
also got corrupted project files etc. But luckily we are past that.

I would like to thank the people who made this happen:

* David Baron for suggesting the initial split. It turned out to be pretty
accurate. David also helped in other ways, and also during the final landing.
* Johnny Stenback for reviews and fixing xpinstall link woes, arranging for the
carpool and checking in most of the stuff (after my laptop died last Tuesday
Johnny did most of the work).
* Peter Van der Beken for the Mac work.
* Chris Waterson for super-review.
* Doug Turner for the help (especially Mac) during the frenzied moments of checkin.
* Other people whose names I do not know during the checkin (Mac fixups etc.),
test crew, and build team...
* And of course people who commented here in the bug + in other instances.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You forgot to mention your mum and dad, without whom, none of this would have
been possible.  =)

Comment 53

19 years ago
Marking verified per last comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.