SpellChecker::markAndReplaceFor is slow |
|||
Issue descriptionThe functions has quadratic running time: O(text length * number of markers). We should make the running time linear.
,
Dec 6 2016
The slowness of Range::boundingBox is due to iterating over all the InlineTextBoxes under a LayoutText, which is hard to optimize. On the other hand, maybe we don't need to updated rendered rectangle for spelling markers. The only client of class RenderedDocumentMarker is FrameView::getTickmarks, which is called by Scrollbar to render the tickmarks for each text match. Hence, I don't think we need to maintain rendered rectangles for other types of document markers.
,
Dec 6 2016
Removing Blink>DOM since we do not consider optimizing Range::boundingBox as a solution. Another idea is to postpone the calculation of rendered rect until needed. Since DocumentMarkerController::renderedRectsForMarkers() is the only direct client of RenderedDocumentMarker, and the function already calls updateMarkerRenderedRectIfNeeded, it is guaranteed to get correct result even if we do not call updateMarkerRenderedRect in DocumentMarkerController::addMarker. Hence, we should be able to safely remove updateMarkerRenderedRect from addMarker.
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f6763007a1508ccbff5f635202c950367769855e commit f6763007a1508ccbff5f635202c950367769855e Author: xiaochengh <xiaochengh@chromium.org> Date: Tue Dec 06 09:30:05 2016 Make calculation of document marker's rendered rect on demand Currently, when adding a document marker, we calculate its rendered rectangle synchronously by calling updateMarkerRenderedRect in DocumentMarkerController::addMarker. This is not needed, since when we retrieve the rendered rectangles (only possible via DMC::renderedRectsForMarkers), updateMarkerRenderedRectIfNeed is also called, calculating the marker's rendered rectangle if we haven't done it before. Hence, this patch removes the updateMarkerRenderedRect call from DMC::addMarker as a performance improvement. BUG= 668408 Review-Url: https://codereview.chromium.org/2557563003 Cr-Commit-Position: refs/heads/master@{#436553} [modify] https://crrev.com/f6763007a1508ccbff5f635202c950367769855e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
,
Dec 7 2016
In retrospect, the time complexity analysis may be less significant than we thought. Though the running time is indeed quadratic, the text length of each request is bounded by kChunkSize=16384 (defined in SpellChecker::chunkAndMarkAllMisspellings). The time spent on text iterating is a product of: - The number of (nodes + lines) of text checked by the request - The number of misspellings which is typically at the order of 10000. As the only observed source of slowness is fixed, we mark this issue as fixed. |
|||
►
Sign in to add a comment |
|||
Comment 1 by xiaoche...@chromium.org
, Dec 6 2016102 KB
102 KB View Download