Closed Bug 77460 Opened 23 years ago Closed 23 years ago

buglists hang mozilla for long periods of time using all available CPU

Categories

(SeaMonkey :: Search, defect, P2)

Product:
SeaMonkey
Component:
Search
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.9

People

(Reporter: asa, Assigned: mozilla)

References

(Depends on 1 open bug)

Details

(Keywords: hang, helpwanted, perf, Whiteboard: [nav+perf][br])

Attachments

(5 files)

examples of the type of string changes I'd like to make (if we had case insensitive searching, which we don't)
5.20 KB, patch
Details | Diff | Splinter Review
same patch I posted earlier, updated to new APIs
5.07 KB, patch
Details | Diff | Splinter Review
partial but internally complete fix for the string problems
15.72 KB, patch
Details | Diff | Splinter Review
more complete patch, but still needs more string code implementation
15.26 KB, patch
Details | Diff | Splinter Review
Don't copy html buffer; just use indexes (no iterators)
17.30 KB, patch
Details | Diff | Splinter Review 
Summary: 
Long buglists (try anything over about 1000, for some real fun try a list of
around 5000 like all resolved fixed bugs) spike the CPU and hang mozilla for a
very long time. 

Tested on:
2001042304 and 2001042404 win32 builds on win2K
2001042408 linux build on redhat6.1
2001042408 mac build on OS9

Steps to reproduce:
1. open browser
2. go to http://bugzilla.mozilla.org/query.cgi and query for Resolved Fixed bugs. 
3. watch your CPU go to the ceiling and sit there for about 5 minutes before
unhanging.

I will test older builds shortly.  This is a recent regression.
Cause is not tables.  82% of the time is spent in
InternetSearchDataSource::webSearchFinalize. 

Of the time in InternetSearchDatasource::webSearchFinalize, almost all is spent
in InternetSearchDatasource::ParseHTML, and:
 * 82% of that is spent in nsAString::Cut, and
 * 13% of that is spent in InMemoryDataSource::Assert
ParseHTML seems to be called only once, so I'm guessing this is a search problem
and not a network problem that's making it be called many times.
I think it would be trivial to convert this code to much faster new-string-code
if the new string code had case-insensitive string searching...
What I've discovered so far is that we've had a CPU spike on long buglists for
some time.  This was reported a few days back as bug 75933.  For the search
Resolved Fixed (about 5100 bugs) the hang is about 25 seconds on my wi32 machine
(PIII 550 with 128 MB RAM).  This hang has been around since at least late last
month (I see it in 3/27 builds) and seems to persist to today.  But something
changed on 4/10.  With the am builds I see the known 25 second hang and all
returns to normal after that hang.  On the evening builds from the 10th I see
the 25 second hang with CPU pegged and that drops down to normal but then
returns to 99% or 100% for almost  11 minutes before the browser becomes
responsive again.  In all of the builds since the 10th I see a 25 second hang at
99 or 100% cpu, a quick dip in cpu, and then a return to 99 or 100% cpu for
another 10 to 11 minutes.  

All tests performed on windows 2000 with win32 talkback builds using the classic
theme.  
Changing component to Search and cc:ing other interested parties.  What could've
started triggering the search stuff?
Component: HTMLTables → Search
meant to reassign too
Assignee: karnaze → matt
QA Contact: amar → claudius
Suggest bug 75933 and this are dups.

Since this bug has more activity, probably best to tag 75933 as the dup.
erik, as I explained in my comments above, I don't think these are the same
thing. The older bug was about a hang that never lasted for more than about 30
seconds.  This hang lasts over 10 minutes for me on a reasonable query.  This is
also newer.  I believe that 75933 is still happening and masked somewhat by this
much uglier hang.
what probably changed was that we stopped hanging at the end of HTTP connections
to bugzilla.mozilla.org. Previously, the search code would never get a
successful document load documents on bugzilla.mozilla.org, which meant the
search processing code would never be kicked off. 
The search code is also spending 6% of its time in
InMemoryDataSource::LockedAssert and 3.5% of its time in
InMemoryDataSource::HasAssertion.  Adding dependency on bug 35817 as well.
Depends on: 35817
*** Bug 79150 has been marked as a duplicate of this bug. ***
Keywords: perf
nav triage team:

Sorry, Asa, we know this really hoses you. Unfortunately, we don't have the 
resources to look at this at the moment. A possible workaround is to remove the 
bugzilla sherlock file "bugzilla.src" from your "Search Plugins" folder.

Marking nsbeta1- and helpwanted
Keywords: helpwanted, nsbeta1-
I found this problem also troubling. However, if you go to
Edit->Preferences...->Navigator->Internet Search and check the "Search Results"
box off (i.e., avoid displaying the bugs in the sidebar), the queries come back
to normal, so this is something to do with the side bar. I'm not certain if this
is new behavior or not.
I like this patch, but I'd like to get scc's input.. 

david - does the patch make a signifigant speed improvement?
That patch is only partly written, so I don't know yet.  (I should've mentioned
that more clearly earlier.)
Since I've been doing occasional work on this and hope to finish it sometime,
assigning to myself (so that I don't forget about it), although if someone else
wants to work on it feel free to take it.
Assignee: matt → dbaron
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.2
*** Bug 84944 has been marked as a duplicate of this bug. ***
Whiteboard: [nav+perf][br]
Target Milestone: mozilla0.9.2 → mozilla1.0
Target Milestone: mozilla1.0 → mozilla0.9.3
*** Bug 88041 has been marked as a duplicate of this bug. ***
*** Bug 77584 has been marked as a duplicate of this bug. ***
*** Bug 81524 has been marked as a duplicate of this bug. ***
*** Bug 89852 has been marked as a duplicate of this bug. ***
From the dups: 
- another thing that would help speed up the search sidebar would be to use 
outliner instead of tree for the "search results" box.
- the search sidebar code should not eat a large number of processor cycles 
when the search sidebar is not visible.
That patch uses the stuff in bug 82054, and also FindCharInSet, which I haven't
written yet, but hopefully will sometime soon so that I can test it...

It doesn't fix all the string problems, but I stopped partway through in a way
that should compile and that I hope will fix most of the string-copying-related
performance problems.

Maybe we should file a separate bug on making the search panel not do anything
when it's not visible...
*** Bug 89957 has been marked as a duplicate of this bug. ***
Blocks: 71668
Filed bug 91168, Search sidebar compiles result list even when panel is not visible.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
I should also fix the messed up line endings in the file around line 5065.
I suggest transferring your votes to bug 91168.  This one probably isn't going
to happen for another few milestones.
Target Milestone: mozilla0.9.4 → mozilla0.9.5
the trick about unchecking Edit->Preferences...->Navigator->Internet Search and
check the "Search Results" doesn't work for me. I have it unchecked, but a query
for all bugs with status NEW in bugzilla froze me for 15 minutes at 95% CPU and
152MB RAM allocated in the end, before i killed it.
(p3/500, 512MB RAM)
Same search in NC4.* flows content into browser in a short time and the browser
is immediately available afterwards. Why isn't this "topperf"..


*** Bug 99903 has been marked as a duplicate of this bug. ***
*** Bug 100251 has been marked as a duplicate of this bug. ***
*** Bug 100610 has been marked as a duplicate of this bug. ***
Keywords: hang
Did something change? I just ran a query in bugzilla that returned over 6000
hits, and there was no hang after it had rendered. Earlyer that would have
frozen the app for 15 minutes at least.. I usually ended up killing it at that
point..
I'm seeing the same -- with 01092408/Linux, the UI is unblocking instantly. 
[Sometime between 0.9.3 and 0.9.4, it had regressed such that any list with >
100 bugs would cause a very noticable hang.]  

I agree. I just loaded the list of Unconformed which usually hangs me (about 800
bugs) for at leasst 10 seconds and I didn't see any hang. Will investigate further.
Woo-Hoo! Buglist of about 6000 also seems fine. I think I'm satisfied.
Is it also fixed if you open the search sidebar?  Does the search sidebar still
work?
So I guess the bugzilla search plugin is broken. It's fast because it's not
populating the sidebar with search results :\
*** Bug 101678 has been marked as a duplicate of this bug. ***
*** Bug 102223 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.5 → mozilla0.9.6
*** Bug 104070 has been marked as a duplicate of this bug. ***
Milestone 0.9.5 on Linux hangs on long buglists again.. sigh.
Target Milestone: mozilla0.9.6 → mozilla0.9.7
This also horks http://www.scientificamerican.com/news to death (bug 84466)
Should be dup'ed.
*** Bug 108559 has been marked as a duplicate of this bug. ***
*** Bug 109050 has been marked as a duplicate of this bug. ***
Note that bug 91168 has now been fixed, so this should be much less painful.  If
that's not the case, please reopen bug 91168.
Target Milestone: mozilla0.9.7 → mozilla0.9.9
Use CBufDescriptor so that we don't need to copy html buffer.  Use indexes (no
iterators).  Cap any search result at 100 items.
CBufDescriptor is going away when we switch to the new string classes.  In fact,
I thought jag already removed all uses from the tree.
Nope, we need to add .Adopt to ns{C}String first (which is safe now that it's on
nsSharableString) before we can replace the CBufDescriptor code.

/me makes a mental note to do so.
Looking for some r/sr love.  Let's get this into the tree, and it can switch
from CBufDescriptor to .Adopt() when jag is done.  :)
over to rjc.  
his patch is available in the bug.

need reviewers help!
Assignee: dbaron → rjc
Status: ASSIGNED → NEW
Doh, after closer inspection I realized your use of CBufDescriptor here is that
you want a thin string wrapped around the buffer without the string actually
owning the buffer, so basically nsDependentString. Can you give that a go? If
you need any help, please do ask.
Hmm... how does nsDependentString help?  You can't call methods such as Find() 
off of it, for example.
Status: NEW → ASSIGNED
You'd have to use FindInReadable. You can stick to this patch though, and I'll
do the conversion when I do a tree sweep to get rid of CBufDescriptor. If you do
stick with this patch, use nsString + CBufDescriptor instead of nsAutoString,
that'll save you 64*2 bytes on the stack (which you don't need since you're
wrapping another buffer).

Why is there a need for MAX_SEARCH_RESULTS_ALLOWED?
I added MAX_SEARCH_RESULTS_ALLOWED because this patch only cuts down on the bloat 
from multiple string copies, not the time to process huge amounts of data -- with 
10,000+ search results, the browser still goes away for dozens of minutes.  Thus, 
the small cap.

> If you do stick with this patch, use nsString + CBufDescriptor
> instead of nsAutoString


Only nsAutoString takes a CBufDescriptor.

r=/sr=jag then.
*** Bug 117604 has been marked as a duplicate of this bug. ***
+
	// use a CBufDescriptor so that "htmlPage" data isn't copied
+
	CBufDescriptor	htmlPageDecriptor( htmlPage, PR_TRUE, htmlPageSize);
+
	nsAutoString		htmlResults(htmlPageDecriptor);
+
	PRUint32        startIndex = 0L, stopIndex = htmlPageSize;

Can you use nsDependentString here, instead? (Otherwise, jag is just going to
have to re-write this later.)
Or use nsString::Adopt, which exists now.
Oops, just checked in the changes then saw the comments.  I guess I'll take jag
up on his offer (http://bugzilla.mozilla.org/show_bug.cgi?id=77460#c58).

dbaron, do you want this bug reassigned back to you, to track your iterator
changes?  Either way, I think that a bug should be opened up against bugzilla to
have large results returned in a page-like manner ala all other search sites,
instead of as one huge chunk of HTML.
rjc, what do you mean "page-like manner"? Do you mean that you want Bugzilla to
break up bug lists across multiple pages? That would be horrible. 
Asa:  Well, having bugzilla send back multiple megs of HTML which chokes Mozilla
(in terms of parser, memory usage, performance, searching, etc) and takes dozens
of minutes seems pretty bad too.
dbaron: actually, one can't use Adopt here, since CBufDescriptor here is used in
a non-owning manner (aStackBased = PR_TRUE), since |htmlPage| is passed in as a
const PRUnichar*, so nsDependentString is what we want.
rjc, bugzilla should return the results so that the display is best for the
person using the page, not to hack around performance bugs in Mozilla's parser.
 You can't know how big a page is going to be when you are using the web, so
Mozilla has to offer reasonable performance in every case.  For example, you can
change bugzilla but lxr is still going to spew out huge pages.  It is
unreasonable to expect the network to adapt to the browser.
> bugzilla should return the results so that the display is best for the
> person using the page, not to hack around performance bugs in Mozilla's parser.


Exactly, we are in agreement.  Having a huge multi-megabyte HTML page is 
basically impossible for the person to use.  Talk about pain!  And its not just 
about Mozilla -- the same huge HTML page sucks to use in any browser.


> It is unreasonable to expect the network to adapt to the browser.

Give me a break, I have no such expectation. My point is simple: from looking at 
other search engines, auction/shopping sites, etc... they strive to make their 
HTML pages highly usable, and one of the things they do is return a reasonably 
small and manageable # of results per page.

Well, I for one want my bug lists huge, multi-megabyte, and impossible for a
person to use. Could have an item like "results_per_page=nn" in the buglist.cgi
query url ("num=nn" in google), which, if omitted or 0, defaults to everything
on one page like it goes now. Anyway, not much to do with this bug...
> Well, I for one want my bug lists huge, multi-megabyte, and impossible
> for a person to use.

Another happy customer!

dbaron, I'm going to bump this bug back to you in case you want to use it to
track your iterator changes... (if not, close 'er out.)  :)
Assignee: rjc → dbaron
Status: ASSIGNED → NEW
Back to rjc...
Assignee: dbaron → rjc
... and marking FIXED.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
*** Bug 124441 has been marked as a duplicate of this bug. ***
Sure it does, you just have to read the bug. See Comment #33.
This bug is about the 5-minute or so hang due to the search sidebar panel. 
Other issues should be filed as separate bugs.
Product: Core → SeaMonkey
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: