98786 - 320 url clones from css loader

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Suresh Duddi (gone)]

CSS loader parser account for 320 url clones of 1200 url objects created on startup.

* 320 url clones from css Loader
  -  36 LoadSheet : 32 agentsheet, 4 Syncload
  - 279 nsCSSParser
  -   5 unaccounted

Comment 1

[Suresh Duddi (gone)]

Test case used in analysis is
./mozilla -P 'Default User' file:///use/tmp/quit.html

Comment 2

[Suresh Duddi (gone)]

Attachment: Removes 279 url objects on startup with blank page

Comment 3

[Gagan]

Comment on attachment 48631 [details] [diff] [review]
Removes 279 url objects on startup with blank page

Looks ok to me. Just two small nitpick things here--

1. I am assuming you'd remove the printfs before checking in (or surround them by #ifdef DEBUG_dp)

2. In the constructor of URLKey you can now get rid of urlstr variable and directly do mURL->GetSpec(&mSpec) Saves a temporary variable allocation. 
Good stuff! r=gagan

Comment 4

[Johnny Stenback (:jst)]

- dp, please do wrap all:

+    printf("DEBUG-dp ...);

lines in #ifdef DEBUG_dp

- This looks a bit odd:

+    return ((nsCRT::strcasecmp(mSpec, key->mSpec) == 0) ? PR_TRUE : PR_FALSE);

why the "? PR_TRUE : PR_FALSE" ? (I know you didn't write it, but you could get
rid of it ;-)

- Do we need the code you #if 0'ed out or could you remove it from the code? If
it's not needed you might as well delete the code to clean things up...

Other than that, sr=jst

Comment 5

[Suresh Duddi (gone)]

So guys, since I do optimize builds to work on performance, printf is my only
recourse and I cannot do DEBUG_dp. I will make *sure* I remove them before
checkin (next time before I attach the patch).

> +  return ((nsCRT::strcasecmp(mSpec, key->mSpec) == 0) ? PR_TRUE : PR_FALSE);
> 
> why the "? PR_TRUE : PR_FALSE" ? (I know you didn't write it, but you could 
> get rid of it ;-)

Done. Changed it to:

+    return nsCRT::strcasecmp(mSpec, key->mSpec) == 0;

> - Do we need the code you #if 0'ed out or could you remove it from the code?
> If it's not needed you might as well delete the code to clean things up...


Done.

Thanks jst. All review comments accepted and will make changes before checkin.

Comment 6

[Suresh Duddi (gone)]

Gagan, thanks. Will do your (1) and (2) review suggestions too.

Comment 7

[Suresh Duddi (gone)]

Fix checked in.