If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsID::Parse() called 1200 times and uses PR_sscanf()

RESOLVED FIXED in mozilla0.8

Status

()

Core
XPCOM
P3
normal
RESOLVED FIXED
18 years ago
17 years ago

People

(Reporter: Suresh Duddi (gone), Assigned: Daniel Bratell)

Tracking

({perf})

Trunk
mozilla0.8
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fix in hand which reduces startup time by 1.6%)

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
Parsing the ID uses PR_sscanf to parse out the ID. It is called 1200 times on 
startup mostly from nsComponentManagerImpl::PrePopulateRegistry()

Function	%	#calls	Time(millesecs)
------------------------------------------------
nsID::Parse	100	1186	56.38

We dont need to fix this for beta. The gain is very minor.
(Reporter)

Updated

18 years ago
Blocks: 28964

Updated

18 years ago
Status: NEW → ASSIGNED

Updated

18 years ago
Target Milestone: M17

Updated

18 years ago
Target Milestone: M17 → M20

Comment 1

18 years ago
mass re-assigning to my new bugzilla account
Assignee: scc → scc
Status: ASSIGNED → NEW

Updated

18 years ago
Status: NEW → ASSIGNED

Comment 2

17 years ago
Updating QA Contact
QA Contact: leger → asa
(Assignee)

Comment 3

17 years ago
I have a fix for this which reduces per call time from 25.96 to 0.43 ms. That 
should save 1.6% of startup time which I noticed is prioritized right now. 

(So now I've done my bit. If anyone else also can shave 1% of the startup time 
we will soon have a browser that's started before the user clicked the icon. :-
) ).

I want a review asap so that I can get this in for Mozilla 0.8. Scott? jband?

scc, can I steal the bug?
Keywords: perf
(Assignee)

Updated

17 years ago
Keywords: mozilla0.8, patch, review
Whiteboard: fix in hand which reduces startup time by 1.6%
(Assignee)

Comment 4

17 years ago
Created attachment 24312 [details] [diff] [review]
Patch that uses custom parsing instead of scanf

Comment 5

17 years ago
sr=jband. 

Looks good to me. Seems to do the right thing for me on NT4. 

FWIW... I just did a quantify run and see that the count of calls to this 
method is 2561 just to startup and quit. This is on a run that does not need to 
do autoreg (where I know the count would be higher). This is twice the count of 
calls it was when I first whined to dp about this.

Thanks!

Comment 6

17 years ago
This looks pretty good.

One nit is inconsistent use of whitespace around operators in this bit:

+    if(the_char>='0' && the_char <= '9') the_int_var -= '0'; \
+    else if(the_char>='a' && the_char <='f') the_int_var -= 'a'-10; \
+    else if(the_char>='A' && the_char <='F') the_int_var -= 'A'-10; \

r=jag.
(Assignee)

Comment 7

17 years ago
Yes, it's called 1200 times by PlatformPrePopulateRegistry and 1200 times by
xptiManifest::Read. Maybe the PlatformPrePopulateRegistry
didn't exist before.
Assignee: scc → bratell
Status: ASSIGNED → NEW
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 8

17 years ago
Patch checked in with some modification to whitespace.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

17 years ago
Or, after reading the first post, maybe the xptiManifest::Read didn't exist before.

Updated

17 years ago
Keywords: mozilla0.8
You need to log in before you can comment on or make changes to this bug.