Closed
Bug 537547
Opened 15 years ago
Closed 15 years ago
Cycle collector uses virtual methods
Categories
(Core :: XPCOM, defect, P3)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: smaug, Assigned: dbaron)
Details
(Keywords: perf)
Attachments
(2 files)
9.33 KB,
patch
|
Details | Diff | Splinter Review | |
8.49 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
Based on Shark profiles GraphWalker::DoWalk is a hot method, yet it calls virtual ShouldVisitNode and VisitNode methods. We should and could avoid virtual methods. One option is to just implement ::DoWalk in each class (using a macro). There are actually just 2 such classes which are compiled normally. Implementing it twice allows (hopefully) compiler to inline ShouldVisitNode and VisitNode.
Assignee | ||
Comment 1•15 years ago
|
||
It wouldn't surprise me if MSVC already inlines both implementations into separate instantiations of DoWalk (though perhaps it only does one, guarded with a vtable pointer check, and lets the other use virtual calls).
Assignee | ||
Updated•15 years ago
|
Summary: Cycle collecter uses virtual methods → Cycle collector uses virtual methods
Reporter | ||
Comment 2•15 years ago
|
||
Comment on attachment 419793 [details] [diff] [review] WIP Well at least OSX gcc doesn't seem to inline anything here. Is this too ugly? We really need to start optimizing CC. After the patch things like nsQueue::Push shows up in the profiles. I think it could be made a bit faster by using faster mod, since as far as I see it is guaranteed that divider is n^2. Or something else than nsQueue could be used in CC. ...but that is a different bug.
Attachment #419793 -
Flags: review?(dbaron)
Assignee | ||
Comment 3•15 years ago
|
||
I guess I think that templates are probably less ugly than macros here; does this provide equivalent performance?
Reporter | ||
Updated•15 years ago
|
Attachment #419793 -
Flags: review?(dbaron)
Reporter | ||
Comment 4•15 years ago
|
||
Comment on attachment 419825 [details] [diff] [review] template-based approach >+ // copy-constructing the visitor should be cleaner than useng a reference useng And I don't know why copy-ctor is cleaner than a reference, but if it is, then ok. Seems to provide similar performance as the macro based patch. And this is easier to read.
Attachment #419825 -
Flags: review+
Reporter | ||
Comment 5•15 years ago
|
||
David, are you going to land your patch?
Assignee | ||
Comment 6•15 years ago
|
||
I just pushed it to try with some other patches; I'll hopefully land it in the next few days (unlikely before Monday, though).
Assignee | ||
Comment 7•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/16e05af6610d
Assignee: nobody → dbaron
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: perf
Priority: -- → P3
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in
before you can comment on or make changes to this bug.
Description
•