373694 - Cycle collector makes Release about 11x slower

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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.  :(

Comment 1

[Graydon Hoare :graydon]

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.

Comment 2

[Peter Van der Beken [:peterv]]

Marking this fixed (as a result of fixing bug 373693).