Closed Bug 105482 Opened 21 years ago Closed 18 years ago

replace nsCRT:: calls with their C-runtime equivalents

Categories

(Core :: XPCOM, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 124536
Future

People

(Reporter: dougt, Assigned: cathleennscp)

Details

(Keywords: helpwanted)

Attachments

(3 files)

No need to be going trough a C++ class to do standard clib stuff.  this will
also allow further decoupling of clients and xpcom.
If the calls are inline, it shouldn't be a problem, at it would allow us to use
better versions when the standard library ones aren't what we want (e.g.,
broken, slow, etc.).  This isn't like NSPR where everything goes through an
extra level of function call overhead.
inlining them will be perf win too. I dont check if they are already so or not.
Here is the info on # calls for startup.

function             calls
--------------------------
nsCRT::strncmp      149800
nsCRT::strncasecmp   97665
nsCRT::HashCode      26047
nsCRT::strcmp        24115
nsCRT::HashCode      14061
Patch anyone?
Target Milestone: --- → mozilla0.9.7
dp: Are those the char* versions (which should already be inlined, but should
probably be changed to call the C runtime versions rather than the PL_ versions)
or the PRUnichar* versions (which we wrote ourselves)?
I tend to agree with dbaron.  similar arguments have been made for nspr. 
Marking as wont fix. 
dp, please reopen if you find that these functions are not inlined.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WONTFIX
Reopening.  They're not inlined since they call to nspr rather than the C
library.  Many compilers have very powerful optimizations for the C library calls.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Keywords: helpwanted
Target Milestone: mozilla0.9.7 → Future
Just these two for now. It doesn't seem to affect pageload times much, but 
second window open times went from a median of 381 to 370.
Comment on attachment 62696 [details] [diff] [review]
Use ::strcmp and ::strncmp inline with assertion

ISTR some c library having the paramaters as char* rather than const char*. IF
that is the case, you'll need to cast

r=bbaetz otherwise
Attachment #62696 - Flags: review+
"Nice".

Anyway, when I tested with a more recent build, second window open times
(median) went from 311 to 310ms (w2k), so I'm not gonna bother with this for now.
over to cathleen.  I saw that you started working on this.  I am not sure if it
is worthwild... but here ya go.
Assignee: dougt → cathleen
Status: REOPENED → NEW
I did some benchmarking with the startup test. gcc 2.95.3 on FreeBSD 4. These
are the numbers (warm):

before: 	   5684 6442 5442
with this patch:   5626 5525 5626
inlining the same: 5683 5643 5534

The numbers does not say too much. Anyone that can do a more reliable
benchmark?

I will attach the inline patch below.
Inline version. 

The previous attachment is not very clean I noticed, sorry about that.
You can't do that.

F:\build\mozilla\content\html\document\src>type 0.cpp
#include <stdio.h>
int main(void){
printf ("%d", strncasecmp); return 0;
}

F:\build\mozilla\content\html\document\src>cl 0.cpp
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 12.00.8168 for 80x86
Copyright (C) Microsoft Corp 1984-1998. All rights reserved.

0.cpp
0.cpp(3) : error C2065: 'strncasecmp' : undeclared identifier

If you want PL_ functions to inline to c library functions, please consider 
asking nspr to make them available as macros.

for example: 
http://www.google.com/search?q=cache:qTHohgX5-IsC:ssobjects.sourceforge.net/doc
s/html/msdefs_8h-source.html+strcasecmp+msvc&hl=en
Dup to 124536? That is where the action about this issue takes place!

*** This bug has been marked as a duplicate of 124536 ***
Status: NEW → RESOLVED
Closed: 21 years ago18 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.