replace nsCRT:: calls with their C-runtime equivalents

RESOLVED DUPLICATE of bug 124536

Status

()

RESOLVED DUPLICATE of bug 124536
17 years ago
14 years ago

People

(Reporter: dougt, Assigned: cathleennscp)

Tracking

({helpwanted})

Trunk
Future
x86
Windows 2000
helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
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.

Comment 2

17 years ago
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
(Reporter)

Comment 3

17 years ago
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)?
(Reporter)

Comment 5

17 years ago
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
Last Resolved: 17 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 → ---
(Reporter)

Updated

17 years ago
Keywords: helpwanted
Target Milestone: mozilla0.9.7 → Future

Comment 7

17 years ago
Created attachment 62696 [details] [diff] [review]
Use ::strcmp and ::strncmp inline with assertion

Comment 8

17 years ago
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+

Comment 10

17 years ago
"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.
(Reporter)

Comment 11

17 years ago
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

Comment 12

17 years ago
Created attachment 69972 [details] [diff] [review]
Replaces nsCRT::str* with C-equivs.

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.

Comment 13

17 years ago
Created attachment 69973 [details] [diff] [review]
Inlines the same stuff as above

Inline version. 

The previous attachment is not very clean I noticed, sorry about that.

Comment 14

17 years ago
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

Comment 15

14 years ago
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
Last Resolved: 17 years ago14 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.