Closed
Bug 97815
Opened 23 years ago
Closed 3 years ago
Font statement is causing performance problems
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
WORKSFORME
Future
People
(Reporter: ire0, Unassigned)
References
()
Details
(Keywords: memory-footprint, perf, testcase, Whiteboard: [residual style])
Attachments
(3 files)
I have a large customer testcase that has the following statement after the body
statement <font face="ARIAL"> .
Removing the font statement helps response time by more than 4 seconds (26 secs
drops to 21.5). I know they don't have
a </font> statement; however, when I default to Ariel without the font
statement I still save the 4 seconds.
In addition, if I have 10 <font face="ARIAL"> statements in a row I add 40+
seconds to my time (I am running on a 333 Mhz OS/2 and a 333 MHz NT machine).
The testcase is a large unordered list.
Also, if I change the font statement to use a make believe font name <font
face="XYZ"> it still costs me 4 seconds for every statement if I leave out the
</font>. 10 font face statements (with out the /font) cost me over 40 seconds
additional. Why should a font statement with an invalid font face cost me so
much time? Why should multiple badly coded font statements cost so much?
Reporter | ||
Comment 1•23 years ago
|
||
I tried the same tests using 4.7 and found no performance problems. The problem
has been around at least since .601 version.
The important thing to note here is if there is no font statement and I default
the same font I improve over 4 seconds. If a put a </font> in right before
</body> I still get charged 4 seconds for the font statement. Why
should the cost be anything more than the time it takes to parse the statement
if using it isn't the cost?
Comment 2•23 years ago
|
||
Is it the case that only *unmatched* <font> tags are causing this slowdown?
Reporter | ||
Comment 3•23 years ago
|
||
If I place the </font> just before the </body> I get the same results.
Comment 4•23 years ago
|
||
Data - Putting some printf's into some of the methods seeming to be
involved,Below is only snippet of trace and only some methods have had
trace printf added. There's probably more going on than in this one
area, but this one area stood out to me.... (anyway, enough
disclaimers....)
WithOUT a font statement get sequences (hundreds/thousands) as such:
07:53:24.200 HTMLStyleSheetImpl::SetAttributeFor
07:53:24.200 HTMLAttributesImpl::SetAttributeFor $Y.
07:53:24.200 HTMLAttributesImpl::SetAttributeName $Y.
07:53:24.200 (exit) HTMLAttributesImpl::SetAttributeFor $Y.
07:53:24.200 (exit) HTMLStyleSheetImpl::SetAttributeFor
WITH a font statement get sequences (hundreds/thousands) as such:
07:53:37.360 HTMLStyleSheetImpl::SetAttributeFor
07:53:37.360 HTMLAttributesImpl::SetAttributeFor $Y.
07:53:37.360 HTMLAttributesImpl::SetAttributeName $Y.
07:53:37.360 HTMLAttributesImpl::EnsureSingleMappedFor
07:53:37.360 nsHTMLMappedAttributes::*constructor*
07:53:37.360 nsHTMLMappedAttributes::Init
07:53:37.360 (exit) HTMLAttributesImpl::EnsureSingleMappedFor
07:53:37.360 nsHTMLMappedAttributes::SetAttribute
07:53:37.360 (exit) nsHTMLMappedAttributes::SetAttribute
07:53:37.360 HTMLAttributesImpl::UniqueMapped »jÿ«j
07:53:37.360 HTMLStyleSheetImpl::UniqueMappedAttributes
07:53:37.360 (exit) HTMLStyleSheetImpl::UniqueMappedAttributes
07:53:37.360 (exit) HTMLAttributesImpl::UniqueMapped »jÿ«j
07:53:37.360 (exit) HTMLAttributesImpl::SetAttributeFor $Y.
07:53:37.360 (exit) HTMLStyleSheetImpl::SetAttributeFor
It's really unclear to me what's going awry but, as a guess, appears a
set of attribute mappings are being many times created/discarded and
recreated,rather than reused, throughout the parsing of the document.
Sam
Comment 5•23 years ago
|
||
"Clarence (Andreas M. Schneider)" <c@c07.de>
suggested:
"Try this:
<style> font { border: 1px solid red } </style>
<font>
<p>text</p>
<p>text</p>
<p>text</p>
and you'll see why it's expensive."
Indeed, give it a try!
My questions are - Is this "valid HTML"?
The text renders differently on NS 4.x, Mozilla, and Opera. (try it!)
Which is 'correct rendering'? or maybe..... why is mozilla choosing to
implement what is clearly the most expensive implementation of this bit
of html?
Sam
Comment 6•23 years ago
|
||
Comment 7•23 years ago
|
||
Comment 8•23 years ago
|
||
If the last <p> is unclosed, behavior is like in testcase 1, even if <font> is
closed. All examples are invalid HTML.
Reporter | ||
Comment 9•23 years ago
|
||
Comment 10•23 years ago
|
||
Data - using ye'ol HTML Reference Manual (previously
http://www.sandia.gov/sci_compute/html_ref.html but removed from there
a year or two ago, is there on-line pointer to it now?)
So then, load time reported by mozilla status time on 700MHz Thinkpad,
html_ref 'as is': 2.3 sec
Add 1 <font face> 3.7 sec
Add 2 <font face> 5.6 sec
Add 3 <font face> 7.4 sec
Add 4 <font face> 9.1 sec
Add 5 <font face> 11.2 sec
...
Add 10 <font face> 20.5 sec
Seems pretty clear.
Sam
Comment 11•23 years ago
|
||
Oh, forgot to add, same html_ref <1 second netscape 4.7 in all cases.
Sam
Comment 12•23 years ago
|
||
Is the fix for bug 96031 going to change the behavior here?
Comment 13•23 years ago
|
||
Apparently, fix for bug 96031 does seem to change the behavior. Will have to
discuss with Marc about it.
Comment 14•23 years ago
|
||
Ivan, do you mean that the problem exists in 6.01? If so, that would mean it
had to exist prior to my style system rewrite.
Reporter | ||
Comment 15•23 years ago
|
||
The problem does exist in 6.01 (using the OS/2 version for my information).
Reporter | ||
Comment 16•23 years ago
|
||
Possible fix for 2 performance bugs (106513 and 97815)
Problem: Large text documents that use style tags such as <font> run much
faster
When carriage returns (\r) and endlines (\n) are removed. By placing nine
<font>
Tags immediately after the <body> tag in my large text document the rendering
slowed by over
10 seconds. Removing the all carriage returns / endlines speeded up the
performance by the full 10
seconds. A single <font> costs about a second but I used 9 to show how the
problem scales.
Possible fix: I tracked down a test in CNavDTD::OpenTransientStyles as the
cause of the slowdown.
nsresult CNavDTD::OpenTransientStyles(eHTMLTags aChildTag){
nsresult result=NS_OK;
// No need to open transient styles in head context - Fix for 41427
if((mFlags & NS_PARSER_FLAG_ENABLE_RESIDUAL_STYLE) &&
eHTMLTag_newline!=aChildTag && mOpenHeadCount==0) {
By removing the eHTMLTag_newline!=aChildTag from the code I got a major
performance
Benefit. I did not notice other changes because of this fix. I do not
understand why
the test for newline is there, only that it dramatically slows down some of my
tests.
I don't know if removing the test will cause other problems; however, it
should be investigated.
I can create the same performance results by generating a whitespace token and
adding
it to the mTokenDeque after placing a newline token on the mTokenDeque or
replacing
endline tokens with whitespace tokens.
I assume that a fix in this area will benefit any documents using Residual style
tags such as
Font, small, tt , etc.
Ivan
Comment 17•23 years ago
|
||
harishd?
Comment 19•23 years ago
|
||
*** Bug 106513 has been marked as a duplicate of this bug. ***
Comment 20•23 years ago
|
||
Comment 21•23 years ago
|
||
harish, not sure what your latest (comment#20) meant to mean. That
seems to be the place the questionable
if(eHTMLTag_newline!=aChildTag)
was added.....
but the code all around this has changed a whole lot since then.
Sam
Comment 22•23 years ago
|
||
Sam: Sorry! I just added the checkin to show that it was added for a
reason...but unfortunately that checkin wasn't documented at all.
Comment 23•23 years ago
|
||
>but unfortunately that checkin wasn't documented at all.
Yea, with sadness, I noticed that too ;-)
Allow me to step back and recap what we've found on this.
This bug (97815) opened when we found multiple font/style/attribute
changes, even if identical to previous style (thus not even really a
change), each adds significant time to document.
Later, we got into investigating why it was the case that for some
cases we were getting great response time improvement from use of
'html cleaner/stripper' utilities. Such utilities just compress out
non-formatting blanks, CRLF, and comments from html source without
altering content. In some cases the performance improvement from use
such utilities was quite surprisingly large.
Then, to our amazement and confusion, we found these two things somehow
related. When html source is stripped of newline line ends, the
performance problem with multiple style changes goes away.
Ivan isolated the single line of code that somehow is crux of the
problem. Removing the newline conditional in that line of code makes
the stripped and unstripped performance/behavior largely the same.
The CRLFs being stripped are no way related to doc latout (no more than
line ends in C++ source code related to compiled C++ execution), so its
a quite surprising result to find these things somehow related.
The suggested code change may not be the best/right change, but at
least gives another clue to the problem.
Comment 24•23 years ago
|
||
Sam: When you mention CRLFs do you really mean CR-LF combo?
Comment 25•23 years ago
|
||
By CRLF, I really meant generic line ending char(s), depending on what
platform, text editor, html generator, etc, created the file. Outside
of preformated text block, CR or LF or CRLF are all equivalent, I
think.
Sam
Comment 26•23 years ago
|
||
Perhaps I'm missing something here, but it sounds to me like the problem is that
within an open <font> tag (incorrectly extended over blocks, quirks mode, blah
blah), residual style handling kicks in. These line endings between tags are
treated as whitespace by the residual style handler, and mayhem ensues.
Theoretically, I suppose this could be dealt with by a full fix of bug 2750 if
we treated misnestings like <p> in <font> as "improper subelements," but I'm
sure trying to apply this bit of SGML pedantry in quirks mode would lead to
disaster.
Comment 27•23 years ago
|
||
Christopher, not sure this fits into your theory, but if the all
instances of line end chars are replaced by blanks then performance
problem goes away. Problem seems unique to line end chars, not
whitespace.
Sam
Comment 28•23 years ago
|
||
Perhaps there's a difference between the handling of whitespace tokens and
newline tokens that there shouldn't be?
Updated•23 years ago
|
Keywords: mozilla0.9.7,
testcase
Comment 29•23 years ago
|
||
that's of course why bug 106513 was duped against this bug (comment 19)
Updated•23 years ago
|
OS: Windows NT → All
Comment 31•23 years ago
|
||
I'm assuming this won't make 0.9.8. There are lots of pages with lots and lots
of unclosed <font> tags, and on long pages apparently this makes a significant hit.
Keywords: mozilla0.9.7 → mozilla0.9.9
Reporter | ||
Comment 32•23 years ago
|
||
The problem is not unclosed <font> tags. Its any residual style (font is a
residual style) that has lots of heirarchical tags (like <ul> tags within <ul>
tags. I found that placing a single blank (future whitespace token) after the
first <ul> speeded up my performance by seconds. I am investigating how a
whitespace tag is processed in CNavDTD::OpenTransientStyles. It seems to prevent
many OpenContainer call on the font node. I am not sure how this causes slowdowns
down stream when we are formatting/reflowing.
Comment 33•23 years ago
|
||
Out of time. Mass move to 0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Updated•23 years ago
|
Keywords: mozilla1.0+
Updated•23 years ago
|
Hardware: PC → All
Comment 35•23 years ago
|
||
*** Bug 142143 has been marked as a duplicate of this bug. ***
Updated•23 years ago
|
Target Milestone: mozilla1.1alpha → mozilla1.1beta
Comment 37•23 years ago
|
||
*** Bug 99435 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 38•23 years ago
|
||
I revisited this bug today using the 1.0.1 nightly. The problem is still there.
When I place a single blank after the first UL in my testcase
(http://bugzilla.mozilla.org/attachment.cgi?id=54915&action=view) my
time drops 2 full seconds (from 17.3 to 15.3). This problem also went away when
I removed the eHTMLTag_newline!=aChildTag from the CNavDTD::OpenTransientStyles
code. See line 2910 for existing code.
nsresult CNavDTD::OpenTransientStyles(eHTMLTags aChildTag){
2906 nsresult result=NS_OK;
2907
2908 // No need to open transient styles in head context - Fix for 41427
2909 if((mFlags & NS_DTD_FLAG_ENABLE_RESIDUAL_STYLE) &&
2910 eHTMLTag_newline!=aChildTag && mOpenHeadCount==0) {
There has not been any activity on this bug for months.
Thanks,
Ivan
Updated•23 years ago
|
Keywords: mozilla0.9.9 → mozilla1.2
[was 1.1beta]
Target Milestone: mozilla1.1beta → Future
Comment 40•22 years ago
|
||
regarding comment #32 shouldn't we change the bug summary?
Comment 41•22 years ago
|
||
*** Bug 165860 has been marked as a duplicate of this bug. ***
Comment 42•22 years ago
|
||
*** Bug 168636 has been marked as a duplicate of this bug. ***
Comment 43•21 years ago
|
||
I'm seeing the same DOM constructed whether or not there are newlines in the
source.... I wish someone had attached the testcase where removing newlines had
n effect to this bug. :(
Whiteboard: residual style
Updated•21 years ago
|
Whiteboard: residual style → [residual style]
Comment 44•20 years ago
|
||
*** Bug 243325 has been marked as a duplicate of this bug. ***
Comment 45•17 years ago
|
||
Markus in comment #40
> regarding comment #32 shouldn't we change the bug summary?
what do you suggest?
FWIW - Ivan (reporter) writes "I haven't looked at the browser stuff in 6 years. Not sure this is still a problem."
Updated•15 years ago
|
Assignee: harishd → nobody
Status: ASSIGNED → NEW
QA Contact: chrispetersen → layout
Comment 46•15 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100503 Firefox/3.6.4
Unable to reproduce using Firefox 3.6.4.
Comment 47•13 years ago
|
||
This bug doesn't applies to Firefox 7.
Comment 48•3 years ago
|
||
Marking this as Resolved > Worksforme since the issue doesn't happen on Windows 10 on either of the Firefox versions.
Also last comments suggest this issue might have been fixed long time ago.
If anyone is still able to reproduce this please re-open or file a new issue.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•