Closed
Bug 393902
Opened 17 years ago
Closed 7 years ago
can we avoid invalidating the entire tree in nsAutocompleteController::ProcessResult()?
Categories
(Toolkit :: Autocomplete, defect, P2)
Toolkit
Autocomplete
Tracking
()
RESOLVED
INVALID
People
(Reporter: moco, Unassigned)
References
()
Details
(Keywords: perf, Whiteboard: [Snappy:P2])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
can we avoid invalidating the entire tree in nsAutocompleteController::ProcessResult()?
spun off from bug #389593 comment #4:
gavin wrote:
Seems like we invalidate the entire tree in ProcessResult(), ideally in the
incremental search case we could invalidate only the rows that were added
(which in most cases won't even be drawn because they're scrolled off the
screen). I think the best way to do this would be to expose additional
invalidate*() variants on nsIAutoCompletePopup, implement them in
autocomplete.xml (which just forwards to the tree's boxobject), and then have
the autocomplete controller invalidate only the parts that have changed. This
might be tricky because the controller can't know how the two sets of results
differ, because the history service nsIAutocompleteSearch impl just calls
onSearchResult multiple times with a new autocomplete result each time, as far
as I can tell, but something to look at in a followup bug, for sure (I think it
would also benefit other autocomplete consumers, and probably fix bug 391889).
Reporter | ||
Comment 1•17 years ago
|
||
see also dietrich comment at https://bugzilla.mozilla.org/show_bug.cgi?id=389491#c32
Comment 2•15 years ago
|
||
adding tsnap to the whiteboard to get this visible, since it affects our ability to make things appear responsive in a reasonable way.
Priority: -- → P2
Whiteboard: [tsnap]
Comment 3•15 years ago
|
||
A work in progress. I am not presently sure how to handle invalidateRow for the richlist popup, which is sadly what the location bar uses. The other one is super easy to do!
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Updated•15 years ago
|
Attachment #388092 -
Attachment is patch: true
Attachment #388092 -
Attachment mime type: application/octet-stream → text/plain
Comment 4•15 years ago
|
||
I don't even know if this works, but I'm uploading a checkpoint. I have figured out how to invalidate just a row with the richlistbox implementation.
Attachment #388092 -
Attachment is obsolete: true
Comment 5•15 years ago
|
||
I am not actually sure that this is even needed. The richlistbox autocomplete smartly updates things. The tree autocomplete doesn't so much though, so it might be needed there.
Comment 6•15 years ago
|
||
This sounds like an opportunistic perf/responsiveness win, so definitely wanted if there's a real win to be had - but not something we'd hold the release for.
Flags: wanted1.9.2+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Updated•13 years ago
|
Whiteboard: [tsnap] → [snappy]
Updated•13 years ago
|
Whiteboard: [snappy] → [Snappy:P2]
Comment 7•7 years ago
|
||
tree autocomplete has gone.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•