Closed
Bug 105482
Opened 23 years ago
Closed 19 years ago
replace nsCRT:: calls with their C-runtime equivalents
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 124536
Future
People
(Reporter: dougt, Assigned: cathleennscp)
Details
(Keywords: helpwanted)
Attachments
(3 files)
910 bytes,
patch
|
bbaetz
:
review+
|
Details | Diff | Splinter Review |
1.73 KB,
patch
|
Details | Diff | Splinter Review | |
1.89 KB,
patch
|
Details | Diff | Splinter Review |
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•23 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
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•23 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
Closed: 23 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•23 years ago
|
Keywords: helpwanted
Target Milestone: mozilla0.9.7 → Future
Comment 7•23 years ago
|
||
Comment 8•23 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 9•23 years ago
|
||
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•23 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•23 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•22 years ago
|
||
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•22 years ago
|
||
Inline version. The previous attachment is not very clean I noticed, sorry about that.
Comment 14•22 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•19 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
Closed: 23 years ago → 19 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•