Cycle collector makes Release about 11x slower

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: bzbarsky, Assigned: peterv)

Tracking

({perf})

Trunk
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Typical numbers from a profile of the testcase in bug 320020:

752 hits under nsDocument::Release, of those 683 under 
nsCycleCollector_suspect.

2222 hits under nsGlobalWindow::Release, of those 2035 under nsCycleCollector_suspect.

In all, 3395 hits are under nsCycleCollector_suspect out of 62777 total hits.

Of the 15185 hits under nsScriptSecurityManager::CheckPropertyAccessImpl, 2464 are under Suspect().

Under Suspect() things look like this:

In Suspect() itself: 481 hits.
Under nsCycleCollector_isScanSafe: 2164 hits (mostly under QI).
Under NS_IsMainThread_P: 424 hits.

Is there anything we can do to make things better here?  5% of total time on DHTML is pretty nontrivial...  Yeah, we should make CheckPropertyAccessImpl do less Release-ing, but still...

I should note that between this bug and bug 373693 this code is taking up about 8% of time on that testcase.  And it seems to me that most of this is debug-like code.  For example, the isScanSafe check could really be an assert, imo.  Same for the main-thread checks...  The stats code could be debug-only.  That sort of thing.  AddRef and Release are kind of on the critical path performance-wise, unfortunately, so even small wins in here count a lot.  :(
Flags: blocking1.9?
Yes. As noted in bug 373693, the main thread check can probably fall into DEBUG-only code. Likewise the isScanSafe check, if you like. The collector was written with a fair amount of on-by-default debugging machinery, which we can disable or make optional.

Updated

12 years ago
Flags: blocking1.9? → blocking1.9+
(Assignee)

Updated

12 years ago
Assignee: nobody → peterv

Updated

12 years ago
(Reporter)

Updated

12 years ago
Blocks: 377787
(Reporter)

Updated

12 years ago
Depends on: 373693
(Assignee)

Comment 2

12 years ago
Marking this fixed (as a result of fixing bug 373693).
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.