Closed Bug 105482 Opened 23 years ago Closed 20 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: 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 → ---
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.
Attached patch Inlines the same stuff as above (deleted) — Splinter Review
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: 23 years ago20 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: