Closed Bug 1449828 Opened 2 years ago Closed 2 years ago

Don't calculate selection rect if unnecessary

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- wontfix
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

When profiling bug 1448818, spellchecker spends a lot of time to calculate selection rect.  But it might be unnecessary with willHaveOverflowingSelection = true
Comment on attachment 8963450 [details] [diff] [review]
Don't calculate selection rect if unnecessary

When profiling spellchecker issue of bug 1448818, CombineSelectionUnderlineRect spends a lot of time.  If didHaveOverflowingSelection is true, it is unnecessary to calculate selection rect.  So we shouldn't call CombineSelectionUnderlineRect every time.
Attachment #8963450 - Flags: review?(jfkthame)
Blocks: 1419519, 1444742
Blocks: 1403521
Keywords: regression
Attachment #8963450 - Flags: review?(jfkthame) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b2ddedc7f5
Don't calculate selection rect if unnecessary. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/e3b2ddedc7f5
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8963450 [details] [diff] [review]
Don't calculate selection rect if unnecessary

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1403521

[User impact if declined]:
When spellchecker is tuned on textarea (such as wikipedia's edit page), if ther e is a lot of misspell words, moving caret by key operation such as arrow key is slow due to high CPU usages.

[Is this code covered by automated tests?]:
Yes and no.  But this is followed by existed reftests for spellchecker.

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
No

[List of other uplifts needed for the feature/fix]:
N/A

[Is the change risky?]:
Low

[Why is the change risky/not risky?]:
We had unnecessary rectangle calculation for condition.  So we doesn't run it if unnecessary.

[String changes made/needed]:
No
Attachment #8963450 - Flags: approval-mozilla-beta?
User's situation is bug bug 1448818 comment #3
Comment on attachment 8963450 [details] [diff] [review]
Don't calculate selection rect if unnecessary

Simple fix for what looks like a pretty easily-hit performance cliff based on the dependent bugs. Let's take it for 60.0b9.
Attachment #8963450 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
No longer blocks: 1419519
Duplicate of this bug: 1419519
No longer blocks: 1444742
Duplicate of this bug: 1444742
(In reply to Makoto Kato [:m_kato] from comment #5)
> [Is this code covered by automated tests?]:
> Yes and no.  But this is followed by existed reftests for spellchecker.
> 
> [Has the fix been verified in Nightly?]:
> Yes
> 
> [Needs manual test from QE? If yes, steps to reproduce]: 
> No

Setting qe-verify- based on Makoto Kato's assessment on manual testing needs and the fact that this fix has (some) automated tests.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.