define nspr_use_zone_allocator to void the symbol lookup

RESOLVED FIXED

Status

()

Firefox
General
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Boying Lu, Unassigned)

Tracking

Trunk
Sun
Solaris
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
Reproduce steps:

1. Using the debugging tool builtin solaris runtime linker:
   LD_DEBUG=symbols,detail LD_DEBUG_OUTPUT=linking.log ./firefox
   to dump some runtime linking information which is stored in linking.log.*

2. In the "linking.log.*" (where the number is the pid), I found that the symbol   
   "nspr_use_zone_allocator" is not found (actually it is not defined at all).

NSPR will search this symbol in the whole process to determine if it should use another memory allocator or not.

For firefox/thunderbird, this symbol is not defined. In this case, runtime linker will search whole process including all loaded shared libraries to find this non-exist symbol.

This causes performance drop when firefox starts up.
(Reporter)

Comment 1

12 years ago
Created attachment 252578 [details] [diff] [review]
define nspr_use_zone_allocator to PR_FALSE to improve startup performance
Attachment #252578 - Flags: review?
(Reporter)

Updated

12 years ago
Attachment #252578 - Flags: review? → review?(wtchang)

Comment 2

12 years ago
Comment on attachment 252578 [details] [diff] [review]
define nspr_use_zone_allocator to PR_FALSE to improve startup performance

r=wtc.  I suggest that you add a short comment to explain
why we define this global variable.
Attachment #252578 - Flags: review?(wtchang) → review+
(Reporter)

Comment 3

12 years ago
Created attachment 253285 [details] [diff] [review]
patch with some comments
Attachment #253285 - Flags: superreview?
(Reporter)

Updated

12 years ago
Attachment #253285 - Flags: superreview? → superreview?(mconnor)
Comment on attachment 253285 [details] [diff] [review]
patch with some comments

sr=mconnor.  How much of an impact does this have, and do we want to consider taking this for the branches if its a nontrivial win?
Attachment #253285 - Flags: superreview?(mconnor) → superreview+
Perhaps this would be a better comment:

/** 
 * NSPR will search for the "nspr_use_zone_allocator" symbol throughout
 * the process and use it to determine whether the application defines its own
 * memory allocator or not.
 *
 * Since most applications (e.g. Firefox and Thunderbird) don't use any special
 * allocators and therefore don't define this symbol, NSPR must search the
 * entire process, which reduces startup performance.
 *
 * By defining the symbol here, we can avoid the wasted lookup and hopefully
 * improve startup performance.
 */
(Reporter)

Comment 6

12 years ago
Created attachment 254277 [details] [diff] [review]
patch with new comments

Comment 7

12 years ago
Checking in toolkit/xre/nsAppRunner.cpp;
/cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.154; previous revision: 1.153
done

=> FIXED.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 8

12 years ago
This has caused red on fx-win32-tbox.

Comment 9

12 years ago
I didn't notice any changes in perf numbers.
(Reporter)

Comment 10

12 years ago
(In reply to comment #9)
> I didn't notice any changes in perf numbers.
> 

This patch doesn't improve the startup performance significantly.
But it does help to improve the startup performance. 

Everytime when firefox starts up, it will lookup the symbol throughout the process
by calling dlsym(). Defining this symbol will help ld.so to find it at the very beginning.

This can save some time when starting up.
You need to log in before you can comment on or make changes to this bug.