Open
Bug 466300
Opened 16 years ago
Updated 2 years ago
Reduce QI and AddRef/Release in nsFind.cpp
Categories
(Core :: Find Backend, defect)
Core
Find Backend
Tracking
()
NEW
People
(Reporter: smaug, Assigned: smaug)
References
Details
(Whiteboard: p=0)
Attachments
(1 file, 1 obsolete file)
11.61 KB,
patch
|
Details | Diff | Splinter Review |
Based on a Shark profile AddRef/Release causes significant overhead
when using type-ahead-find, especially if the page doesn't contain the searched
word.
Patch coming.
Assignee | ||
Comment 1•16 years ago
|
||
Attachment #349581 -
Flags: superreview?(peterv)
Attachment #349581 -
Flags: review?(peterv)
Comment 2•16 years ago
|
||
Comment on attachment 349581 [details] [diff] [review]
this should help a bit
It should be fairly easy to make SkipNode and IsBlockNode take nsINode (GetParent would return null for attributes and documents). I would also make GetBlockParent take a nsIContent and do the QI in the one caller.
Assignee | ||
Comment 3•16 years ago
|
||
I'm not trying to convert everything to use nsINode/Content, just fixing some
obvious problems, mainly GetBlockParent.
If SkipNode/IsBlockNode took nsINode*, they would have to have similar
nsINode* to nsIContent* casting that in the patch is done in one place, in
GetCurrentContent().
Not sure why the caller should QI, not GetBlockParent. That shouldn't matter at all. There is anyway one QI, unless someone does bigger changes and makes
mIternode to be nsINode* or nsIContent*.
Anyway, the patch is just a fix-something-while-quickly-glancing-over-some-unowned-code-during-a-train-trip :)
Assignee | ||
Comment 4•16 years ago
|
||
With this the AddRef/Release caused by nsFind drops down from the
top of the Shark profile. Testcase is to search for some non-existing word
in mxr.m.o/nsCSSFrameConstructor.cpp.
Attachment #349581 -
Attachment is obsolete: true
Attachment #349734 -
Flags: superreview?(peterv)
Attachment #349734 -
Flags: review?(peterv)
Attachment #349581 -
Flags: superreview?(peterv)
Attachment #349581 -
Flags: review?(peterv)
Assignee | ||
Comment 6•15 years ago
|
||
The patch does seem to apply cleanly (with --fuzz=4).
Peter, just say if you don't like the approach.
Comment 7•13 years ago
|
||
Peter, do you think you'll review this patch in the near future? I'll help refresh the patch if Olli's approach is fine. Thanks.
Hardware: x86 → All
Assignee | ||
Updated•13 years ago
|
Attachment #349734 -
Flags: superreview?(peterv)
Attachment #349734 -
Flags: review?(peterv)
Updated•11 years ago
|
Whiteboard: [feature] p=0
Updated•11 years ago
|
Blocks: fxdesktopbacklog
Updated•11 years ago
|
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•