Closed Bug 133624 Opened 22 years ago Closed 22 years ago

Txul jumped 15%

Categories

(SeaMonkey :: General, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED DUPLICATE of bug 135046

People

(Reporter: skasinathan, Assigned: morse)

Details

(Keywords: perf, regression)

The latest Txul timings shows a jump by 15%. Tree is closed to investigate this.
4:15 PM PST.
Adding darin, harishd, jrgm, mccafee to cc list.
Adding beard to cc list.

The only checkins in the relevant cycle were darin and harishd, and it sure
looks like harishd:

http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&date=explicit&mindate=1017182640&maxdate=1017184499

Reassigning.
Assignee: cathleen → harishd
Except, I have to say, that really doesn't make much sense either.  (I certainly
hope darin's "remove a blank line" didn't cause this.)
Maybe mcafee was logged on to the machine because he was setting it up to rename
it on the tinderbox page, and that slowed the tests down?  I'm inclined to give
this another cycle and assume it's noise until then.
this might be due to cookies.  i found that cookieTasksOverlay.xul had an extra
newline at the top of the file.  i removed this because it was causing the XML
header text to appear below the status bar in the chrome!  IOW: i suspect
cookieTasksOverlay.xul wasn't being executed/parsed/etc.

cc'ing morse
morse's patch added this line to cookieTasksOverlay.xul:

  if (!("@mozilla.org/cookie-consent;1" in Components.classes)) {

that looks like it might be expensive?!
Oh, comet builds with all extensions, and this re-adds that extra cookie UI that
caused the Txul jump last week to any build that has p3p enabled, doesn't it?
->morse, although I think this is a known issue and only affects builds with
p3p, not all builds.

We should probably decide whether comet should be building with p3p enabled or not.
Assignee: harishd → morse
I *think* p3p is enabled by deafult.
This is morse's comment in his checkin at 12:19 today. "turn on p3p by default".
No, it's not.  configure.in (current) says:

MOZ_EXTENSIONS_DEFAULT=" cookie wallet content-packs xml-rpc xmlextras help pref
transformiix venkman inspector irc"
MOZ_EXTENSIONS_ALL="$MOZ_EXTENSIONS_DEFAULT xmlterm access-builtin ctl p3p
interfaceinfo" 


I don't think this should hold the tree closed.  We know what the problem is,
but somebody needs to decide what to do about it.
I think that checkin comment was misleading -- judging from the bug and the
changes what it should have said is:
  "Making cookie preferences use P3P by default in builds where P3P is enabled."
Keywords: perf, regression
I think we can live with not building p3p on comet. Since planetoid/monkeypox
build p3p anyway.
Keywords: perf, regression
I'm confused here.  Before I checked in I did timing both with p3p dll present 
and without it.  This added line of code did not take any time.

I don't know when or how I inadvertently added that blank line to the file, but 
it was not there when I was doing the timing tests.  If it was, then my timing 
statements which were in that file would not have generated output.
> This is morse's comment in his checkin at 12:19 today. "turn on p3p
> by default".

Let me clarify this.  It means turning on the p3p pref, not building the p3p 
module.  This timing increase is occuring when the module is built and present 
at run time.
What kind of timing?  Were you timing new window open, or just some segment of
code?  Adding new overlays / chrome can make all sorts of things a lot slower --
more overlay merging, more content model construction, more style resolution, etc.
I still didn't say it clearly.  My change is not to build p3p by default.  
Rather it is to use p3p by default if it is present. 
My timing was around specific areas, such as turning on the cookie icon, loading 
the dll, etc.  It did clearly indicate that the increase we saw last week was 
due to the loading of the p3p module when we needed to test for its presense.  
SO the changes that I made today were to test without loading.
OK, I don't understand why the increase occured.

But what I'm hearing from mcaffee and from harishd, is that if this occurs only 
when p3p is part of the build, then we don't need to worry about it.  Recall 
that p3p will not be part of the build in the 1.0 release.

If that's true, can I close this out as won't-fix?
Makes perfect sense to me.  I was just annoyed that the tree was closed because
of this.
Mcafee talked to me and we opened the tree! Thanks McCafee!!
Keywords: perf, regression

*** This bug has been marked as a duplicate of 135046 ***
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → DUPLICATE
vrfy
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.