The default bug view has changed. See this FAQ.

1.5% increase in Ts time, --enable-mathml ?

RESOLVED FIXED in mozilla1.0


15 years ago
12 years ago


(Reporter: Chris McAfee, Assigned: rbs)



Firefox Tracking Flags

(Not tracked)



(5 attachments, 4 obsolete attachments)



15 years ago
Sometime around 6pm tonight, startup time Ts
increased across the 6 tinderboxes running Ts test
about 1.5% or so.

Comment 1

15 years ago
Created attachment 68706 [details]
startup increase graph

Comment 2

15 years ago
Started looking here, 3:30 checkin list.

Hmm, --enable-mathml ?  Maybe this got turned on correctly and is
taking up startup time now?
None of the jumps on comet correspond to --enable-mathml.

Comment 4

15 years ago
cc'ing everyone who checked in, r=, sr= or wrote the patches checked in during
that time.  If we are holding the tree closed for this bug, we need some urgency
My jsstr.c change removed locking code, so it could only speed up string match,
replace, or split.  My jsregexp.[ch] changes affect code that doesn't run at
startup, based on breakpointing in gdb.

My changes only affect mouse clicks on form elements, so they wouldn't be hit
during a startup test.

Comment 7

15 years ago
more detailed check-in logs from all ts test machines

openwound 15063->15125  (+62ms)

mecha 6423->6472->6495  (+72ms total)

facedown 6341->6412     (+71ms)

comet 1616->1626        (+10ms)

sleekstack 5447->5497   (+50ms)

Comment 8

15 years ago
Just took out one unused ``_elementID'' from pref-debug.xul.  Can't have
affected startup time.

Comment 9

15 years ago
mail/news code doesn't run at startup, so Navin and Darin's changes would not
affect startup time.

Comment 10

15 years ago
There were 3 jumps in Ts Friday.

One of them was around 16:10 and the only checkin around that time that
seems to have anything to do with startup is danm's. I'm going to try backing
it out to test the effect on ts. (while the tree is closed). I mailed him
earlier but he seems to be away.

02/08/2002 16:10 mozilla/xpfe/appshell/src/nsAppShellService.cpp
1.175 6/8  generally give windows a 'minimize' widget. bug 77020 r=ben,jag

Comment 11

15 years ago
blake backed this out and two cycles passed on comet and Ts stayed
the same.

I just now backed out 2/8/2002 danm's 14:34 checkin of 
mozilla/ xpfe/ appshell/ src/ nsWebShellWindow.cpp
mozilla/ xpfe/ appshell/ src/ nsWebShellWindow.h

Comment 12

15 years ago
These are the changes i made. backing this stuff out didn't 
make any differece to startup times.

cvs update -j1.175 -j1.174 mozilla/xpfe/appshell/src/nsAppShellService.cpp
cvs update -j1.123 -j1.122 mozilla/xpfe/appshell/src/nsWebShellWindow.h
cvs update -j1.368 -j1.367 mozilla/xpfe/appshell/src/nsWebShellWindow.cpp

I tried backing out stuff that seemed likely to have caused
the problem but that didn't work and i'm not very inclided 
to back out the entire day's worth of stuff. If someone wants
to continue that then fine, otherwise i think i should just 
put everything back where i found it.

I'll check this back in in two hours unless someone vetos the idea

Comment 13

15 years ago
checked danm's stuff back in. everything should be back to normal now.

bryner is pulling and testing by date.
I measured a startup time increase of about 1% (17.5ms) on my machine when
MathML was enabled.

Comment 15

15 years ago
Could that just that the binaries including MathML are somewhat bigger? In that
case it could be a price we have to pay for that standard. 
mathml was only enabled on unix at 2002/02/08 23:46:37 according to cvs log -
the original changes were wrong, and rbs backed them out. The real
changes to define mathml only went in then.
bbaetz: I think 2002/02/08 23:46:37 is GMT, thus 02/08/2002 15:46 local time as
displayed in the bonsai queries.
Also note that the "real" checkin turning mathml on was the "Automated update"
of configure that went in at 02/08/2002 15:57, showing up as leaf's checkin.

This checkin show up in all the suspicious time intervals, so it certainly
explains the jump.
Created attachment 68781 [details]
Size diffs of >100 Bytes per file between 2002-02-08-{08,21}

The most significant increase in size is libgklayout with ~230KB, as expected.
With this, enabling mathml cannot be done with zero cost. (But I'm not saying
that all of the delta is unavoidable.) component.reg is another obvious one.

Compare these numbers with attachment 64118 [details] in bug 109826 which were generated
from 0.9.7 builds with and without mathml, but keep in mind that they were done
with --disable-elf-dynstr-gc, so the size diffs for .so files should be smaller
now, and also mathml has had quite a few checkins between 0.9.7 and now.

Comment 20

15 years ago
There is something that I can think of re:mathml. At compile time, it adds the
mathml.css stylesheet to the ua.css:

 37 @import url(resource:/res/html.css);
 38 @import url(chrome://global/content/xul.css);
 39 @import url(resource:/res/quirk.css);
+   @import url(resource:/res/mathml.css);

(This happens with a little perl script

But the mathml.css style rules are scoped under the mathml namespace, and rules
are hashed by namespace, so these rules should be isolated in a bucked somewhere
preventing other elements from being tested against them (dbaron, that was
bug 35847, right?)

For background, must read:

The other thing that I can think of is that mathml adds new atoms (which all go
in the single global table that everybody shares). This is needed to test which
tags are encountered in a page -- e.g., how to know if <math> is encountered?
Could the impact of extra atoms be a significant factor in 1% ?

Apart from these two, the mathml code by itself is lazy and does its setup on
demand (when the user visits a page with <math>...</math> for the first time).

Comment 21

15 years ago
A fact that is clear, there are side-effects from the libs (old libs that have 
slightly augmented sizes, and new libs -- ucvmath).

Are the outputs of the timeline-enabled builds logged somewhere so that we can 
check the delta from the moment where mathml was enabled? That would be hightly 
informative indeed.

(in particular, I am curious to see if ucvmath is coming into play, I am 
currently setting up a timeline-enabled build to see what is going on at 

Comment 22

15 years ago
Created attachment 68793 [details]
timeline output - ucvmath doesn't show up at startup

find ucvmath returns no occurrences, doesn't seem like ucvmath is the culprit.

Comment 23

15 years ago
mathml.css does show up here:

00009.174:  nsIOService::NewChannelFromURI(resource:///res/quirk.css)
00009.174: nsIOService::NewChannelFromURI([...]/WIN32_D.OBJ/bin/res/quirk.css)
00009.224:  nsIOService::NewChannelFromURI(resource:///res/mathml.css)
00009.224:  nsIOService::NewChannelFromURI([...]/WIN32_D.OBJ/bin/res/mathml.css)
00010.035:  PR_LoadLibrary total: 6.460 
00010.155:  PR_LoadLibrary total: 6.570 

Comment 24

15 years ago
Didn't find other occurrences of strings with /math/. So culprits might be the 
increased dlls?

Comment 25

15 years ago
I am currenlty lurking in the Style System to see if the rules in mathml.css are 
being used (unexpectedly) during general style resolution.

Comment 26

15 years ago
-> rbs.  Added mathml to summary.
Assignee: cathleen → rbs
Summary: 1.5% increase in Ts time → 1.5% increase in Ts time, --enable-mathml ?

Comment 27

15 years ago
disabled mathml with --disable-mathml on comet, sleestack tinderboxes.
We got about half of the Ts delta back on comet, maybe 75% of the
delta back on sleestack.  I am testing this on facedown, results
in a few hours as that is a slower box.

It is possible that either 1) --disable-mathml isn't completely
disabling this, or 2) there is another regression hiding under the
mathml smokescreen.

Comment 28

15 years ago
Created attachment 68803 [details]
output during style processing while mathml.css is there

The namespace hashing seems to be doing well. The rules in mathml.css are left
out. But mathml.css is coming into play when initializing the UA stylesheets
(see the lines annotated with [MATHML] -- they are grouped together -- they all
happen when the style system is loading and initializing its data -- but I am
not sure why the namespace ID is zero everywhere).

Could you try to add an early exit in so that it doesn't execute
anything (i.e., it doesn't add mathml.css to the ua.css)? If there is a win by
doing that, it might mean that the addition of mathml.css is a hot spot worth
investigating, and so strategies like that used to load viewsource.css on
demand might have to be explored here.

Comment 29

15 years ago
> 1) --disable-mathml isn't completely disabling this

That option looks improbable given that TB would have turned red as it did the 
first time. Below is the old vs new before & after --enable-mathml. 
(Build gurus, is something missing?)


dnl ========================================================
dnl ========================================================
[  --enable-mathml         Enable MathML ],


dnl ========================================================
dnl MathML on by default
dnl ========================================================
[  --disable-mathml      Disable MathML],
if test "$MOZ_MATHML"; then

Comment 30

15 years ago
Trying all sorts of voodoo, I disabled mathml.css (which timeline says is 
currently the sole thing that is directly loaded at startup as a result of
--enable-mathml). Let's see if that makes a difference.

Comment 31

15 years ago
Not much consistent gain on the TB outputs. Seems to be just noise.
Is Ts measuring a cold start?
Has anyone measured warm starts?

Comment 33

15 years ago
--disable-mathml recovers about 80% of the Ts delta,
this matches sleestack's behavior.  Comet didn't show
as much time recovery, the I/O is much much faster
than both sleestack and facedown so maybe that is
part of the reason.

Comment 34

15 years ago
Since removing mathml.css didn't seem to do much good, and there is no other 
loaded thing at startup that is mathml-only, the increase comes from the 
increased libs, right? 
That's why you want to try warm starts. The impact of increased lib size is
smaller with warm starts.
rbs -- comet and sleestack had already had mathml turned off with
--diable-mathml when you checked in your patch, so it wouldn't have had any effect.

Comment 37

15 years ago
All tinderbox perf tests first fire the app up for the AliveTest,
e.g. does this crash on startup.  Then the tests run; for the Ts
case this means starting the app up 10 times, and we report the
best time of all those tries.

Comment 38

15 years ago
Could you just try to leave mathml.css out (this way, it would be clear that it 
is worth investigating the route of loading it on demand).

Comment 39

15 years ago
tinderbox deletes dist for each build pass, is that good enough?
Are we still pushing mathml.css out even though it's disabled?

Comment 40

15 years ago
No it is left out (since the directory where it is housed isn't compiled and the 
perl script isn't launched).

But what is missing is the delta from the increased dlls (the price to pay), and 
other side-effects that can be removed with a bit a work (although not simple). 

In the case of viewsource.css, it applied to most thing (HTML namespace), so a 
win could be anticipated. But in the case of mathml.css, it is already confined 
to the mathml namespace, so it is unclear whether it might help to delay it or 
not. That's why the question is: is there any win with the increased 
mathml-enabled dlls but without the unnecessary mathml.css? If the anwser is yes 
then, mathml.css could somehow be loaded on demand, if doable.

Comment 41

15 years ago
All the 'idef MOZ_MATHML' hooks:

Comment 42

15 years ago
(... shows that ucvmath was yet again forgotten in intl/uconv/ etc )
Is anyone looking at what caused the _rest_ of the startup time increase? I'm
pretty sure it wasn't all mathml.

Comment 44

15 years ago
Loading on demand isn't that hard.  You just wait until an element is 
encountered during XML content node buildup that matches the MathML 
namespace, and at that point synchronously load the agent sheet using 

Comment 45

15 years ago
MathML has no hook in the content code. I wasn't very keen to add yet another
hook there at this stage, and/or was instead looking at it from the DOCTYPE or
from the frame construction code. I might indeed try lazy loading of it for
sanity sake. Did you people noticed an improvement when it was left out?

Comment 46

15 years ago
Will comment back later about the lazy loading of mathml.css - will have a go at 
intercepting the doctype, and see how it goes, or try other things as necessary.

Comment 47

15 years ago
Created attachment 68993 [details] [diff] [review]
patch to load mathml.css on demand (first solution)

The patch checks the doctype and loads the file if its FPI (Formal Public
Identifier) is the MathML one. Since there is only one doctype per document,
its cost is negligible since it just adds a single |if| test per page load when
MathML is enabled.

I have a second solution that slightly extends the patch so that it doesn't
need an #idef at all, and can be applied to others (SVG) in an extensible way.
But that iteration requires changing existing APIs.

Comment 48

15 years ago
The second solution is to extend the catalog table used in nsExpatDriver as 

struct nsCatalogEntry {
  const char* mPublicID;
  const char* mLocalDTD;
+ const char* mAgentSheet; // where in the future this can ultimately be an
                           // nsISupports* mCataloData for something bigger
                           // (see bug 98413: Implement XML Catalogs)

Then reconfigure the catalog table to read:

static const nsCatalogEntry kCatalogTable[] = {
 {"-//W3C//DTD XHTML Basic 1.0//EN",           "xhtml11.dtd", nsnull },

 {"-//W3C//DTD XHTML 1.1 plus MathML 2.0//EN", "mathml.dtd",
                                               "resource:/res/mathml.css" },

 {"-//W3C//DTD SVG 20001102//EN",              "svg.dtd", nsnull },

 {nsnull, nsnull, nsnull}

Then change nsIExpatSink.idl so that

nsXMLContentSink::HandleDoctypeDecl(const PRUnichar *aDoctype,
                                    PRUint32 aLength)
and its other friends become

nsXMLContentSink::HandleDoctypeDecl(const PRUnichar *aDoctype,
                  PRUint32 aLength,
                  PRUnichar* aCatalogData)

where, for the moment, the catalog data would just be the pointer to the 
additional agent sheet needed for the corresponding XML vocublary. And do
  if (aCatalogData) { // additional agent sheet set
    load it...

That's basically the idea, it just touches lots of files. I am fine with with 
either solution.

Comment 49

15 years ago
Created attachment 69207 [details] [diff] [review]
second solution (allow to load catalog data on demand)

harishd, heikki, this patch affects mostly the XML area. What is happening is
explained in my earlier post. In order to implement this, I needed to keep
track of the catalog entry associated toe current FPI (otherwise another lookup
would have been needed). So I added a mCatalogData member variable, and
reshuffled the code around to simple initialize it when available:

+	 if (aFPIStr) {
+	   // see if the Formal Public Identifier (FPI) maps to a catalog entry

+	  mCatalogData = LookupCatalogData(aFPIStr);
+	}

In summary, I turned Driver_HandleExternalEntityRef() into a wrapper in a
similar to the other drivers, and cached the catalog entry when it is
processed. Then, passed the data along at the stage when the doctype handler of
the sink is called. It is much clear to see the full picture upon applying the

I am more inclined towards this second solution since it removes the #ifdef and
makes another little step towards fixing bug 98413. However, since it crosses 
other modules, I am okay with either solution that other module owners prefer.
The most impacted remains MathML since mathml.css is being parsed all the time.

Comment 50

15 years ago
Comment on attachment 69207 [details] [diff] [review]
second solution (allow to load catalog data on demand)

Since mainstream users don't need to load mathml.css upfront, I am asking r=
and sr= so that this little gem can bake in the tree a little while before the
freeze for m0.9.9.

Comment 51

15 years ago
Comment on attachment 69207 [details] [diff] [review]
second solution (allow to load catalog data on demand)

>Index: content/xml/document/src/nsXMLContentSink.cpp
>RCS file: /cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v
>retrieving revision 1.205
>diff -u -r1.205 nsXMLContentSink.cpp
>--- content/xml/document/src/nsXMLContentSink.cpp	8 Feb 2002 22:46:09 -0000	1.205
>+++ content/xml/document/src/nsXMLContentSink.cpp	13 Feb 2002 06:13:44 -0000
>@@ -68,6 +68,7 @@
> #include "nsHTMLParts.h"
> #include "nsVoidArray.h"
> #include "nsCRT.h"
>+#include "nsIStyleSet.h"
> #include "nsICSSLoader.h"
> #include "nsICSSStyleSheet.h"
> #include "nsIHTMLContentContainer.h"
>@@ -1459,7 +1460,8 @@
> nsXMLContentSink::HandleDoctypeDecl(const PRUnichar *aDoctype,
>-                                    PRUint32 aLength)
>+                                    PRUint32 aLength,
>+                                    nsISupports* aCatalogData)
> {

Why should aCatalogData be nsISupports*? It can be nsIURI* ( and therefore can
avoid an extra QI ). no? 

>+          if (!styleSet) continue;

Please break this ( and others ) into two lines. 

>+Driver_HandleExternalEntityRef(void* aExternalEntityRefHandler,
>                                const XML_Char *openEntityNames,
>                                const XML_Char *base,
>                                const XML_Char *systemId,
>                                const XML_Char *publicId)
> {

Could you replace aExternalEntityRefHandler with aUserData ( just like other
wrapper methods )?

Still reviewing......

Comment 52

15 years ago
>Why should aCatalogData be nsISupports*? It can be nsIURI* ( and therefore can
>avoid an extra QI ). no? 

You hit the nail... Actually, I have hesitated between four options at the time 
of writing the patch:
- use a specific typename, e.g., |nsIURI*|, but opted out with the thinking that 
it wouldn't be immediate to a reader that the FPI of a document helps to lookup 
arbitrary built-in catalog data, and that this data can really be any object (or 
set of objects) in the future. (currently: the FPI is already used to determine 
two things -- the additional DTD file and the agent CSS file for formatting the
XML vocabulary being represented)
- use a |PRUnichar* aURL|, and do the NewURI(), & etc, from within the handlers. 
Prone to code duplications in the handlers. 
- use |void*|, then I thought, people would have to carry all the associated 
declarations in order to do casting (i.e., not really XPCOM-friendly)
- use |nsISupports*|, it had my vote because it didn't tie people to anything, 
it didn't impact those that were not actually using the data, and hinted at 
future extensions. The side-effect was that it had the overhead of a QI, but 
this remained limited to the case of an actual consumer and was one-off.

Anyway, these are the four possible options, maybe I am missing something in 
balancing the pros & cons.

> >+          if (!styleSet) continue;
>Please break this ( and others ) into two lines.


>Could you replace aExternalEntityRefHandler with aUserData ( just like other
>wrapper methods )?

Notice that Expat makes a difference. There is a specific setter for this (it 
can set a callback different from the default userdata that the other handlers 
always get). I am not clear why Expat does so. I got confused at first. That's 
why I used a different name so that other people reading the code can stop and 
take notice about this peculiarity too. Do you still prefer to call this 

Comment 53

15 years ago
Created attachment 70700 [details] [diff] [review]
updated patch in sync with the tip


15 years ago
Attachment #68993 - Attachment is obsolete: true
Attachment #69207 - Attachment is obsolete: true


15 years ago
Target Milestone: --- → mozilla0.9.9

Comment 54

15 years ago
-> moving to 1.0, if/when r/sr/a arrive.
Target Milestone: mozilla0.9.9 → mozilla1.0

Comment 55

15 years ago
Noted that the extra agent sheet is lost when going back and forth in print 
preview. Since PP just creates a document viewer and another style set [c.f.
DocumentViewerImpl::CreateStyleSet()] and walks the existing DOM without 
reparsing/reloading the document again, we don't get the chance to add the extra 
agent sheet in PP. The same story happens when PP goes away and the main window 
is recreated. The document is relaid out without the extra agent sheet.

Perhaps, it might be best to add the extra agent sheet straight in
nsContentDLF::gUAStyleSheet if it isn't there already. However, each doc viewer 
clones the UA sheet, and it could be that the extra sheet wasn't there yet when 
the UA was cloned (this might have a ramification in 
DocumentViewerImpl::CreateDocumentViewerUsing() where the UA sheet is passed on 
to another [c.f. viewer->SetUAStyleSheet(mUAStyleSheet)]. So, some other 
trickeries might be needed to fully cover all the edge cases. Need 

Comment 56

15 years ago
rods, care to read comment #55 above.

Comment 57

15 years ago
Created attachment 72730 [details] [diff] [review]
third solution (attach catalog sheets to the document)

There are entaglements out there... and it looked shaky to try to figure out
who (e.g., PP) is doing what with the document and stay in sync with them. So
this patch now makes the document the owner of the catalog stylesheet. The
catalog stylesheet stays alongside the other hidden stylesheets (the attr sheet
& inline sheet). I traced in the debugger and noted that things are going on
smoothly and there is even an early return in PresShell::ReconstructStyleData()
due to the fact that no frame has been constructed, for the catalog stylesheet
is added soon after the document has been created and there are no frames yet.

Seeking r/sr/a. As we are still early in the milestone, unanticipated problems
could be resolved or the patch backed out.
Attachment #70700 - Attachment is obsolete: true


15 years ago
Summary: 1.5% increase in Ts time, --enable-mathml ? → [fix, awaiting r/sr/a(trunk)] 1.5% increase in Ts time, --enable-mathml ?

Comment 58

15 years ago
Created attachment 73472 [details] [diff] [review]
updated path - sync:ing with the tip

Attaching a conflict-free patch in sync with the numerous checkins that have
been going on recently.

I haven't observed a problem so far with this third approach (normal browsing
is OK, mathml.css gets loaded on demand on MathML pages, print-preview &
theme-switching stay okay on MathML pages too).
Attachment #72730 - Attachment is obsolete: true

Comment 59

15 years ago
Comment on attachment 73472 [details] [diff] [review]
updated path - sync:ing with the tip

>Index: content/base/public/nsIDocument.h
>RCS file: /cvsroot/mozilla/content/base/public/nsIDocument.h,v
>retrieving revision 3.124
>diff -u -r3.124 nsIDocument.h
>--- content/base/public/nsIDocument.h	7 Mar 2002 03:33:52 -0000	3.124
>+++ content/base/public/nsIDocument.h	10 Mar 2002 20:49:37 -0000
>@@ -256,13 +256,13 @@
>   NS_IMETHOD GetNumberOfStyleSheets(PRInt32* aCount) = 0;
>   NS_IMETHOD GetStyleSheetAt(PRInt32 aIndex, nsIStyleSheet** aSheet) = 0;
>   NS_IMETHOD GetIndexOfStyleSheet(nsIStyleSheet* aSheet, PRInt32* aIndex) = 0;
>-  virtual void AddStyleSheet(nsIStyleSheet* aSheet) = 0;
>+  virtual void AddStyleSheet(nsIStyleSheet* aSheet, PRUint32 aFlags) = 0;
>   virtual void RemoveStyleSheet(nsIStyleSheet* aSheet) = 0;
>   NS_IMETHOD UpdateStyleSheets(nsISupportsArray* aOldSheets, nsISupportsArray* aNewSheets) = 0;
>   NS_IMETHOD InsertStyleSheetAt(nsIStyleSheet* aSheet, PRInt32 aIndex, PRBool aNotify) = 0;
>   virtual void SetStyleSheetDisabledState(nsIStyleSheet* aSheet,
>-                                          PRBool mDisabled) = 0;
>+                                          PRBool aDisabled) = 0;
Could you please replace |virtual void| with NS_IMETHOD_(void)? ( May be this
is not your problem )

+#ifdef NS_DEBUG
>+      nsCAutoString uriStr;
>+      uri->GetSpec(uriStr);
>+      printf("Loading catalog stylesheet: %s ... %s\n", uriStr.get(), sheet.get() ? "Done" : "Failed");

Optional: You can use nsXPIDLCString instead of nsCAutoString.

Comment 60

15 years ago
>Could you please replace |virtual void| with NS_IMETHOD_(void)? ( May be this
>is not your problem )

Let's not bother with this for now. There are several declarations like that in 
those files and with the associated NS_IMETHODIMP that would be needed for an 
overall consistency, the whole patch will just look more scary for r/sr/a.

>Optional: You can use nsXPIDLCString instead of nsCAutoString.

Debug only, and nsXPIDLCString was in the earlier patch. I removed it to sync 
with the iDNS landing that changed the interface from |char*| to |nsAString&|, 
causing the former |getter_Copies()| to stop the compilation.

Comment 61

15 years ago
Comment on attachment 73472 [details] [diff] [review]
updated path - sync:ing with the tip

Attachment #73472 - Flags: review+


15 years ago
Summary: [fix, awaiting r/sr/a(trunk)] 1.5% increase in Ts time, --enable-mathml ? → [fix, awaiting sr/a(trunk)] 1.5% increase in Ts time, --enable-mathml ?
Whiteboard: have r=harishd
Comment on attachment 73472 [details] [diff] [review]
updated path - sync:ing with the tip

Attachment #73472 - Flags: superreview+


15 years ago
Summary: [fix, awaiting sr/a(trunk)] 1.5% increase in Ts time, --enable-mathml ? → [fix, awaiting a=] 1.5% increase in Ts time, --enable-mathml ?
Whiteboard: have r=harishd → have r=harishd, sr=jst

Comment 63

15 years ago
Comment on attachment 73472 [details] [diff] [review]
updated path - sync:ing with the tip

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73472 - Flags: approval+

Comment 64

15 years ago
Patch checked-in, mathml.css is now loaded on demand rather than at startup.
Note: people wanting to use MathML somewhere outside the XHML+MathML doctype 
(e.g., in the chrome) now need to include mathml.css explicitly.
Last Resolved: 15 years ago
Resolution: --- → FIXED
Summary: [fix, awaiting a=] 1.5% increase in Ts time, --enable-mathml ? → 1.5% increase in Ts time, --enable-mathml ?
Whiteboard: have r=harishd, sr=jst
Is there any evidence that this checkin fixed (a significant part of) the
performance regression? (I looked, but I didn't find one on tinderbox. Maybe I
just didn't look hard enough, or I looked at the wrong place.)

See also comment 43.

Comment 66

15 years ago
I didn't notice much difference at the time of my checkin. From comment #14, the
increase in Ts was about 1% (17.5ms) and it was mostly I/O bound. Since other
things have been happening since then, it is hard to tell. What I did notice for
sure was a reported drop in the bloat numbers on TB. Anyway, it appeals to
reason to load this stuff on demand since it isn't the first cookie that makes
one fat.

Comment 67

15 years ago
See follow-up bug 132844: "Transformed docs lack the DOCTYPE, detect the MathML 

I just checked in a patch to detect the MathML namespace at element creation, 
and load mathml.css from there too. So there is no need to include mathml.css 
explicitly even in documents that don't have a DOCTYPE.

Now if a document has the XHTML+MathML DOCTYPE, mathml.css is speedily loaded
as per this bug, otherwise the first MathML element triggers the load as per
bug 132844 (but since this later case can arise mid-way, a rebuild of the entire
frame tree may arise in the usual layout way).
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.