105482 - replace nsCRT:: calls with their C-runtime equivalents

Status: RESOLVED DUPLICATE of bug 124536

(Core :: XPCOM, defect)

Description

[Doug Turner (:dougt)]

No need to be going trough a C++ class to do standard clib stuff.  this will
also allow further decoupling of clients and xpcom.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[Suresh Duddi (gone)]

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

Comment 3

[Doug Turner (:dougt)]

Patch anyone?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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)?

Comment 5

[Doug Turner (:dougt)]

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.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 7

[jag (Peter Annema)]

Attachment: Use ::strcmp and ::strncmp inline with assertion

Comment 8

[jag (Peter Annema)]

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 9

[Bradley Baetz (:bbaetz)]

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

Comment 10

[jag (Peter Annema)]

"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.

Comment 11

[Doug Turner (:dougt)]

over to cathleen.  I saw that you started working on this.  I am not sure if it
is worthwild... but here ya go.

Comment 12

[Peter A Jonsson]

Attachment: 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

[Peter A Jonsson]

Attachment: Inlines the same stuff as above

Inline version. 

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

Comment 14

[timeless]

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

[Alfred Kayser]

Dup to 124536? That is where the action about this issue takes place!

*** This bug has been marked as a duplicate of 124536 ***