Ts/Txul regression on linux platform resulting from async-io change (bug 176919). strange that windows and mac saw no regression.
i will be investigating into this over the weekend.
Severity: normal → critical
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.3beta
There are those uninited variable warnings.... Windows will set them to 0, and so won't notice any resulting problems.
Created attachment 111927 [details] [diff] [review] patch to make max ODA size match "old" code this is something i figure we should test. bbaetz: thx for noticing the unitialized variables.. i'll look into that ASAP.
i don't think any of the uninitialized variable warnings could explain this problem. they are all benign warnings (except perhaps for the JAR channel one, but that would only get us into trouble if the mime service wasn't around). jrgm: can you try out the patch i submitted? thx!!
running patch on mecca, here: http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey-Testerbox build is currently in testonly mode, e.g. no cvs or build.
I see no change in Ts or Txul numbers on mecca with this patch.
I didn't notice them - they were mentioned in the other bug. I'm wondering if there is some calculation somewhere which win32 is optimising out, but our compiler with -O isn't. Anyone feel like a jprof to look for a function which needs an |inline| added, or something?
Since mcafee beat me to the patch, I thought I'd just confirm what the tinderboxen reported in my own build, with and without darin's carpool landing (i.e., backed it out). Testing on Linux Redhat 6.1, 500MHz, 128MB, I get: Txul -- about 7% slower with the carpool landing Ts -- about 1.5% slower with the carpool landing Tp -- uncached: about 2% faster with the carpool landing cached: about 6% faster with the carpool landing (Actually, it's interesting to see how this patch affects individual pages. The range of results in cached times is from 12% slower to 35% faster across the regular forty test pages, with 32 of 40 pages showing equal or faster times, over half scoring >10% faster. I don't recognize a content-based relationship among the pages that slowed the most, or among the pages the speeded up the most. [i.e., sometimes I can say "Oh, those pages all have a lot of JS" or some such statement, but not this time]). I'm going to set up with jprof with these builds and see if anything obvious pops up. (But I probably won't have anything tonight).
i gave this bug a lot of thought over saturday, and it's no surprise now that the patch i attached above does not improve things. i looked at some nsJarProtocol logs, and i noticed something very interesting. the nsJARChannel is delivering quite a few OnStopRequest events well after the last OnDataAvailable. for some consumers, like the nsCSSLoader (which uses nsUnicharStreamLoader), parsing is delayed until the OnStopRequest. in the old world, the OnStopRequest came quickly after the last OnDataAvailable. in the new world, the nsInputStreamPump controls the ODA/OSR events for the nsJARChannel, and i have some idea of what can be done to force the OSR to immediately follow the last ODA. patch in the works... mcafee, jrgm: thanks so much for all the help!
Created attachment 111987 [details] [diff] [review] revised patch (with early OSR) this patch causes OSR to fire immediately after the last ODA. this should help Ts. on my windows machine (i don't have an optimized linux build handy at the moment), this patch improves Ts by roughly 1.5%. i'm hopeful that it'll make a bigger difference on linux.
Attachment #111927 - Attachment is obsolete: true
i checked in this latest patch to see if it does the trick.
I think this last patch got most of the Txul time back, some of the Ts time. Still, this is a big improvement, thanks Darin.
> majority of the difference er, I mean, that the reason that Ts is not all the way back down is that that other bug also regressed Ts. [Damn, how come a few stupid icons can affect perf that much].
marking FIXED. the other Ts regression is unrelated to my changes.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Yeah, I have builds on linux where one has darin's carpool backed out, and the other has the carpool plus this patch above. I am not seeing any difference in the times recorded for Txul or Ts.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.