Last Comment Bug 665858 - Using Japanese IME, response of input is painfully slow on Wikipedia editing page
: Using Japanese IME, response of input is painfully slow on Wikipedia editing ...
Status: VERIFIED FIXED
: inputmethod, regression
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla7
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
http://ja.wikipedia.org/w/index.php?t...
Depends on: 692145
Blocks: 240933
  Show dependency treegraph
 
Reported: 2011-06-21 03:48 PDT by Alice0775 White
Modified: 2012-01-30 23:47 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1 - Optimize the conversion of native and cross platform text offsets (4.61 KB, patch)
2011-06-22 14:47 PDT, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset (3.78 KB, patch)
2011-06-22 15:34 PDT, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset (3.80 KB, patch)
2011-06-22 17:07 PDT, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review
Part 1 - Optimize the conversion of native and cross platform text offsets (4.73 KB, patch)
2011-06-22 17:59 PDT, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review
Part 1 - Optimize the conversion of native and cross platform text offsets (4.51 KB, patch)
2011-06-23 15:06 PDT, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
masayuki: review+
Details | Diff | Review
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset (3.63 KB, patch)
2011-06-23 15:13 PDT, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Part 1 - Optimize the conversion of native and cross platform text offsets (4.53 KB, patch)
2011-06-24 15:57 PDT, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset (3.61 KB, patch)
2011-06-24 15:58 PDT, :Ehsan Akhgari (busy, don't ask for review please)
masayuki: review+
Details | Diff | Review

Description Alice0775 White 2011-06-21 03:48:51 PDT
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/50b63701fc01
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110620 Firefox/7.0a1 ID:20110620121237

Using Japanese IME, response of input is painfully slow.

*This happens Firefox4.0 and later.
*This does not happen Firefox3.6.x and Google Chrome 12.0.742.100.


Reproducible: Always

Steps to Reproduce:
1. Start Nightly with new profile
2. Open Wikipedia editing page ( Ex. http://ja.wikipedia.org/w/index.php?title=Wikipedia:%E3%82%B5%E3%83%B3%E3%83%89%E3%83%9C%E3%83%83%E3%82%AF%E3%82%B9&action=edit )
3. IME on
4. Key in some text

Actual Results:  
  Response of input is painfully slow.

Expected Results:  
  it should be at usual speed.


Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/68033b993ed7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100913 Firefox/4.0b6pre ID:20100913165011
Fails:
http://hg.mozilla.org/mozilla-central/rev/ccaffbc6a970
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100913 Firefox/4.0b6pre ID:20100913162558
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=68033b993ed7&tochange=ccaffbc6a970

Suspected bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=240933#c117
Relanded Bug 240933 - Plaintext editor should stop using <br> all over
Comment 1 Alice0775 White 2011-06-21 09:43:10 PDT
*This happens at the time of the editing of the huge sentence and texts.

*And it does not happen if IME off.
Comment 2 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-21 13:37:44 PDT
I will profile this today.
Comment 3 Alice0775 White 2011-06-21 16:14:14 PDT
This happens in not only Wikipedia but also ordinary TEXTAREA.

[STR]
1. Start Nightly with New profile
2. Open https://bugzilla.mozilla.org/show_bug.cgi?id=240933
3. Select All and Copy
4. Paste in to TEXTAREA at the bottom of the page. (Paste the huge texts on TEXTAREA)
6. IME ON
7. Key in some text with IME
Comment 4 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-21 16:34:52 PDT
So, I couldn't actually reproduce the bug.  Could you please let me know exactly which keyboard layout to use and what to do in order to reproduce this?  (Are you able to reproduce this on multiple platforms?  I tried to reproduce this on Mac)
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-21 16:35:22 PDT
(Note that I don't speak Japanase, so telling me exactly which keys to press would be most helpful) :-)
Comment 6 Alice0775 White 2011-06-21 17:16:30 PDT
(In reply to comment #4)
> So, I couldn't actually reproduce the bug.  Could you please let me know
> exactly which keyboard layout to use and what to do in order to reproduce
> this?  (Are you able to reproduce this on multiple platforms?  I tried to
> reproduce this on Mac)

This does not happen on Ubuntu10.04+Ibus.
So,I guess this happens only on Windows platform.

(In reply to comment #5)
> (Note that I don't speak Japanase, so telling me exactly which keys to press
> would be most helpful) :-)

1. Using Microsoft IME on Windows7 Japanese Edition and Japanese 109keyboard,
2. Press ALT+半角/全角漢字 key(to start IME ON)
3. Press "w a t a s h i h a" keys without quotation and space
4. Displaying of one by one character is very slow.

** I think you should contact Masayuki Nakano (Mozilla Japan).
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-21 19:28:36 PDT
I can reproduce this too. When I test this bug on debug build, I can see some warnings but I'm not sure this is related or not.

I suspected the nsContentEventHandler performance. But according to the log, it seems not so.

When I type 'A' on the wikipedia's text area, I got following widget log:

> 0[51f220]: IMM32: OnKeyDownEvent, hWnd=00041f20, wParam=000000e5, lParam=00290001
> 0[51f220]: IMM32: OnIMENotify, hWnd=00041f20, IMN_SETOPENSTATUS
> 0[51f220]: IMM32: nsIMM32Handler is created
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 283, y: 646, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED

log for normal case (not reproduced this perf regression)

> 0[51f220]: IMM32: OnKeyDownEvent, hWnd=00041f20, wParam=000000e5, lParam=00040001
> 0[51f220]: IMM32: OnIMEStartComposition, hWnd=00041f20, mIsComposing=FALSE
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 976, y: 170, width: 2, height: 14 }
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 976, y: 170, width: 2, height: 14 }
> 0[51f220]: IMM32: SetIMERelatedWindowsPos, mNativeCaretIsCreated=TRUE, width=2 height=14
> 0[51f220]: IMM32: SetIMERelatedWindowsPos, Set candidate window
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 976, y: 170, width: 2, height: 14 }
> 0[51f220]: IMM32: OnIMENotify, hWnd=00041f20, IMN_SETCANDIDATEPOS, lParam=00000001
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 976, y: 170, width: 2, height: 14 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: HandleStartComposition, START composition, mCompositionStart=0
> 0[51f220]: IMM32: OnIMEComposition, hWnd=00041f20, lParam=000001b9, mIsComposing=TRUE
> 0[51f220]: IMM32: OnIMEComposition, GCS_RESULTSTR=no, GCS_COMPSTR=YES, GCS_COMPATTR=YES, GCS_COMPCLAUSE=YES, GCS_CURSORPOS=YES
> 0[51f220]: IMM32: HandleComposition, GCS_COMPSTR
> 0[51f220]: IMM32: GetCompositionString, SUCCEEDED mCompositionString="あ"
> 0[51f220]: IMM32: HandleComposition, GCS_COMPCLAUSE, useA_API=FALSE
> 0[51f220]: IMM32: HandleComposition, GCS_COMPCLAUSE, mClauseLength=2
> 0[51f220]: IMM32: HandleComposition, GCS_COMPATTR, mAttributeLength=1
> 0[51f220]: IMM32: HandleComposition, GCS_CURSORPOS, mCursorPosition=1
> 0[51f220]: IMM32: DispatchTextEvent, aCheckAttr=TRUE
> 0[51f220]: IMM32: SetTextRangeList, index=0, rangeType=NS_TEXTRANGE_RAWINPUT, range=[0-1]
> 0[51f220]: IMM32: SetTextRangeList, caret position=1
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 976, y: 170, width: 12, height: 14 }
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 987, y: 170, width: 2, height: 14 }
> 0[51f220]: IMM32: SetIMERelatedWindowsPos, Set candidate window
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 976, y: 170, width: 12, height: 14 }
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 976, y: 170, width: 12, height: 14 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMENotify, hWnd=00041f20, IMN_SETCANDIDATEPOS, lParam=00000001
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 976, y: 170, width: 12, height: 14 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 976, y: 170, width: 12, height: 14 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-21 19:31:19 PDT
Oops, first log is wrong, correct log is here:

> 0[51f220]: IMM32: OnKeyDownEvent, hWnd=00041f20, wParam=000000e5, lParam=001e0001
> 0[51f220]: IMM32: OnIMEStartComposition, hWnd=00041f20, mIsComposing=FALSE
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 235, y: 719, width: 1, height: 18 }
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 235, y: 719, width: 1, height: 18 }
> 0[51f220]: IMM32: SetIMERelatedWindowsPos, mNativeCaretIsCreated=TRUE, width=1 height=18
> 0[51f220]: IMM32: SetIMERelatedWindowsPos, Set candidate window
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 235, y: 719, width: 1, height: 18 }
> 0[51f220]: IMM32: OnIMENotify, hWnd=00041f20, IMN_SETCANDIDATEPOS, lParam=00000001
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 235, y: 719, width: 1, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: HandleStartComposition, START composition, mCompositionStart=3138
> 0[51f220]: IMM32: OnIMEComposition, hWnd=00041f20, lParam=000001b9, mIsComposing=TRUE
> 0[51f220]: IMM32: OnIMEComposition, GCS_RESULTSTR=no, GCS_COMPSTR=YES, GCS_COMPATTR=YES, GCS_COMPCLAUSE=YES, GCS_CURSORPOS=YES
> 0[51f220]: IMM32: HandleComposition, GCS_COMPSTR
> 0[51f220]: IMM32: GetCompositionString, SUCCEEDED mCompositionString="ち"
> 0[51f220]: IMM32: HandleComposition, GCS_COMPCLAUSE, useA_API=FALSE
> 0[51f220]: IMM32: HandleComposition, GCS_COMPCLAUSE, mClauseLength=2
> 0[51f220]: IMM32: HandleComposition, GCS_COMPATTR, mAttributeLength=1
> 0[51f220]: IMM32: HandleComposition, GCS_CURSORPOS, mCursorPosition=1
> 0[51f220]: IMM32: DispatchTextEvent, aCheckAttr=TRUE
> 0[51f220]: IMM32: SetTextRangeList, index=0, rangeType=NS_TEXTRANGE_RAWINPUT, range=[0-1]
> 0[51f220]: IMM32: SetTextRangeList, caret position=1
> 0[51f220]: WARNING: Ended font run in the middle of a cluster: 'end == aSource->GetLength() || aSource->IsClusterStart(end)', file m:/mozilla-l/src/gfx/thebes/gfxFont.cpp, line 4229
> 0[51f220]: WARNING: Started font run in the middle of a cluster: 'aSource->IsClusterStart(start)', file m:/mozilla-l/src/gfx/thebes/gfxFont.cpp, line 4227
> 0[51f220]: WARNING: Ended font run in the middle of a cluster: 'end == aSource->GetLength() || aSource->IsClusterStart(end)', file m:/mozilla-l/src/gfx/thebes/gfxFont.cpp, line 4229
> 0[51f220]: WARNING: Started font run in the middle of a cluster: 'aSource->IsClusterStart(start)', file m:/mozilla-l/src/gfx/thebes/gfxFont.cpp, line 4227
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 234, y: 719, width: 17, height: 18 }
> 0[51f220]: IMM32: GetCaretRect, SUCCEEDED, aCaretRect={ x: 251, y: 719, width: 1, height: 18 }
> 0[51f220]: IMM32: SetIMERelatedWindowsPos, Set candidate window
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 234, y: 719, width: 17, height: 18 }
> 0[51f220]: IMM32: OnIMENotify, hWnd=00041f20, IMN_SETCANDIDATEPOS, lParam=00000001
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 234, y: 719, width: 17, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
> 0[51f220]: IMM32: OnIMERequest, hWnd=00041f20, IMR_QUERYCHARPOSITION
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aOffset=0, SUCCEEDED
> 0[51f220]: IMM32: GetCharacterRectOfSelectedTextAt, aCharRect={ x: 234, y: 719, width: 17, height: 18 }
> 0[51f220]: IMM32: HandleQueryCharPosition, SUCCEEDED
Comment 9 Alice0775 White 2011-06-21 22:14:12 PDT
FYI,
In local build(windows7):
build from c611f052ec92 :very slow
build from 0f5a2dce1aa5 : normal speed
Triggered:
c611f052ec92	Ehsan Akhgari — Bug 240933 - Part 1: Do not split multiline text into textframes separated by BR elements; r=roc a=dbaron
Comment 10 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 11:37:51 PDT
OK, I managed to reproduce this.  Thanks for your help.  I'll investigate.
Comment 11 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 13:59:34 PDT
Here is what's happening.

During nsIMM32Handler::DispatchTextEvent, we end up calling nsContentEventHandler::SetRangeFromFlatTextOffset <http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsContentEventHandler.cpp#374> (see the stack below).


 	xul.dll!nsContentEventHandler::SetRangeFromFlatTextOffset(nsIRange * aRange=0x0e1c0620, unsigned int aNativeOffset=86266, unsigned int aNativeLength=1, int aExpandToClusterBoundaries=1)  Line 374 + 0x1b bytes	C++
 	xul.dll!nsContentEventHandler::OnQueryTextRect(nsQueryContentEvent * aEvent=0x0034ce24)  Line 551	C++
 	xul.dll!nsEventStateManager::PreHandleEvent(nsPresContext * aPresContext=0x0dc23110, nsEvent * aEvent=0x0e07083c, nsIFrame * aTargetFrame=0x12dc1eb0, nsEventStatus * aStatus=0x0034cce0, nsIView * aView=0x0e0754f0)  Line 1342	C++
 	xul.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0e070820, nsIView * aView=0x0e0754f0, nsEventStatus * aStatus=0x0034cce0)  Line 7127	C++
 	xul.dll!PresShell::HandleEvent(nsIView * aView=0x0e0754f0, nsGUIEvent * aEvent=0x0034ce24, int aDontRetargetEvents=239428272, nsEventStatus * aEventStatus=0x0034cce0)  Line 6884 + 0x11 bytes	C++
 	xul.dll!PresShell::HandleEvent(nsIView * aView=0x0a1b17f8, nsGUIEvent * aEvent=0x0e050d08, int aDontRetargetEvents=175507928, nsEventStatus * aEventStatus=0x0034cce0)  Line 6561 + 0x10 bytes	C++
 	xul.dll!nsViewManager::HandleEvent(nsView * aView=0x00000000, nsGUIEvent * aEvent=0x0034ce24)  Line 1056	C++
 	xul.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0034ce24, nsIView * aView=0x0a1b17f8, nsEventStatus * aStatus=0x0034cd4c)  Line 1031 + 0x9 bytes	C++
 	xul.dll!AttachedHandleEvent(nsGUIEvent * aEvent=0x09bcabc0)  Line 193	C++
 	xul.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0034ce24, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 3545 + 0x4 bytes	C++
 	xul.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x00000000)  Line 3572	C++
 	xul.dll!nsIMM32Handler::GetCharacterRectOfSelectedTextAt(nsWindow * aWindow=0x00000007, unsigned int aOffset=0, nsIntRect & aCharRect={...})  Line 1871	C++
 	xul.dll!nsIMM32Handler::SetIMERelatedWindowsPos(nsWindow * aWindow=0x09bf7b30, const nsIMEContext & aIMEContext={...})  Line 1926	C++
 	xul.dll!nsIMM32Handler::DispatchTextEvent(nsWindow * aWindow=0x09bf7b30, const nsIMEContext & aIMEContext={...}, int aCheckAttr=1)  Line 1677	C++
 	xul.dll!nsIMM32Handler::HandleComposition(nsWindow * aWindow=0x09bf7b30, const nsIMEContext & aIMEContext={...}, long lParam=441)  Line 1273	C++
 	xul.dll!nsIMM32Handler::OnIMEComposition(nsWindow * aWindow=0x09bf7b30, unsigned int wParam=12431, long lParam=441)  Line 571 + 0xe bytes	C++
 	xul.dll!nsIMM32Handler::ProcessMessage(nsWindow * aWindow=0x09bf7b30, unsigned int msg=271, unsigned int & wParam=12431, long & lParam=441, long * aRetValue=0x0034d270, int & aEatMessage=2004231718)  Line 404	C++
 	xul.dll!nsWindow::ProcessMessage(unsigned int msg=271, unsigned int & wParam=12431, long & lParam=441, long * aRetValue=0x0034d270)  Line 4525 + 0x15 bytes	C++
 	xul.dll!nsWindow::WindowProcInternal(HWND__ * hWnd=0x09bf7b30, unsigned int msg=271, unsigned int wParam=12431, long lParam=441)  Line 4402 + 0x17 bytes	C++
 	xul.dll!CallWindowProcCrashProtected(long (HWND__ *, unsigned int, unsigned int, long)* wndProc=0x67641049, HWND__ * hWnd=0x000c0086, unsigned int msg=271, unsigned int wParam=12431, long lParam=441)  Line 65 + 0xf bytes	C++
 	xul.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000c0086, unsigned int msg=271, unsigned int wParam=12431, long lParam=441)  Line 4344 + 0x16 bytes	C++

ConvertToXPOffset is really inefficient.  It first copies the text node string, then truncates it, then goes ahead and replaces native newlines to XP newlines.  This means that if the textnode has N newlines, we perform N memmoves, making an O(N^2) algorithm, which hurts us badly now that textareas can contain huge text nodes.

Actually none of this is necessary at all!  On Mac (where the length of the native newline is equal to the length of the XP newline), we can just return the native offset.  On Windows, we can calculate the number of newlines in the string, and return aNativeOffset - numberOfNewLines.
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 14:47:51 PDT
Created attachment 541185 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets
Comment 13 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 15:34:03 PDT
Created attachment 541199 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

nsContentEventHandler::GetFlatTextOffset also showed pathological performance characteristics.  This function too is O(N^2), and also does a bunch of unnecessary copying, which can be avoided because we're only interested in the length, and not the actual strings.
Comment 14 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 15:35:42 PDT
With these two patches, the performance of entering Japanese text with IME turned on is the same as entering English text for me.  In the xperf profile, painting seems to be a larger chunk of the time spent while typing characters, which makes me quite happy!
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-22 15:45:24 PDT
Comment on attachment 541185 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Review of attachment 541185 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-22 15:50:05 PDT
Comment on attachment 541199 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

Review of attachment 541199 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/events/src/nsContentEventHandler.cpp
@@ +262,5 @@
>      return 0;
> +  if (aMaxOffset == 0xffffffff) {
> +    aMaxOffset = 0;
> +  }
> +  return text->GetLength() - textLengthDifference - aMaxOffset;

I don't understand this line. You seem to be returning the length of the text after aMaxOffset, but shouldn't you be returning the length before aMaxOffset?

At least document what aMaxOffset means.
Comment 17 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 17:07:08 PDT
Created attachment 541235 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

(In reply to comment #16)
> ::: content/events/src/nsContentEventHandler.cpp
> @@ +262,5 @@
> >      return 0;
> > +  if (aMaxOffset == 0xffffffff) {
> > +    aMaxOffset = 0;
> > +  }
> > +  return text->GetLength() - textLengthDifference - aMaxOffset;
> 
> I don't understand this line. You seem to be returning the length of the
> text after aMaxOffset, but shouldn't you be returning the length before
> aMaxOffset?
> 
> At least document what aMaxOffset means.

Good catch, you're right!
Comment 18 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-22 17:59:44 PDT
Created attachment 541251 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

GetNativeTextLength can accept BR nodes in addition to text nodes, so asserting that the node parameter is a text node is a mistake.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-22 21:58:32 PDT
Comment on attachment 541235 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

Review of attachment 541235 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-22 21:59:02 PDT
Comment on attachment 541251 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Review of attachment 541251 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 21 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-23 15:06:51 PDT
Created attachment 541517 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Sorry for the numerous iterations here, but I discovered two bugs with the previous version of this patch.

The first bug was with the newline counting.  I incorrectly had assumed that the input to CountNewlinesIn has native newlines, so I was looking for them.  But it's actually the other way around, so I just need to look for linefeed characters.
Comment 22 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-23 15:07:52 PDT
The second bug was in GetNativeTextLength.  Since we're getting the native length, we should be adding the adjustment value calculated instead of subtracting it from the original (XP) length.
Comment 23 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-23 15:13:35 PDT
Created attachment 541521 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

This part has only changed to apply on top of part 1.  It's probably a good idea for Masayuki to review these patches too, since it's now apparent that my coding skills are less than perfect.  ;-)
Comment 24 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-23 15:14:18 PDT
Comment on attachment 541521 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

...so are my bugzilla usage skills!
Comment 25 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-23 21:40:58 PDT
Comment on attachment 541517 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Review of attachment 541517 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/events/src/nsContentEventHandler.cpp
@@ +216,5 @@
> +               "aContent is not a text node!");
> +  const nsTextFragment* text = aContent->GetText();
> +  if (!text)
> +    return 0;
> +  if (aMaxOffset == 0xffffffff) {

Why don't you define name const or macro for 0xffffffff?

@@ +246,5 @@
> +    CountNewlinesIn(aContent, 0xffffffff);
> +#else
> +    // On other platforms, the native and XP newlines are the same.
> +    0;
> +#endif

I think the lines in this #if/#elif/#else should be indented one more level.
Comment 26 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-23 21:42:57 PDT
(In reply to comment #25)
> @@ +246,5 @@
> > +    CountNewlinesIn(aContent, 0xffffffff);
> > +#else
> > +    // On other platforms, the native and XP newlines are the same.
> > +    0;
> > +#endif
> 
> I think the lines in this #if/#elif/#else should be indented one more level.

This is about GetNativeTextLength().
Comment 27 Boris Zbarsky [:bz] 2011-06-23 21:46:12 PDT
> Why don't you define name const or macro for 0xffffffff?

PR_UINT32_MAX?
Comment 28 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-23 22:07:15 PDT
Comment on attachment 541521 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

Review of attachment 541521 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/events/src/nsContentEventHandler.cpp
@@ +254,5 @@
>        return 0;
> +    PRUint32 length = text->GetLength();
> +    if (aMaxOffset != 0xffffffff) {
> +      length = aMaxOffset;
> +    }

If aMaxOffset is larger than actual length but aMaxOffset isn't 0xffffffff, it causes some bugs. Maybe, it should be:

PRUint32 length = NS_MIN(text->GetLength(), aMaxOffset);

And aMaxOffset may be misleadable. Isn't aMaxLength better? (CountNewlinesIn()'s aMaxOffset too.)
Comment 29 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-06-23 23:06:29 PDT
Comment on attachment 541517 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Review of attachment 541517 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/events/src/nsContentEventHandler.cpp
@@ +216,5 @@
> +               "aContent is not a text node!");
> +  const nsTextFragment* text = aContent->GetText();
> +  if (!text)
> +    return 0;
> +  if (aMaxOffset == 0xffffffff) {

as bz said, PR_UINT32_MAX
Comment 30 Mozilla RelEng Bot 2011-06-24 03:01:12 PDT
Try run for 5c4767395a8b is complete.
Detailed breakdown of the results available here:
    http://tbpl.mozilla.org/?tree=Try&rev=5c4767395a8b
Results:
    exception: 3
    success: 142
    warnings: 16
Total buildrequests: 161
Comment 31 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-24 15:57:44 PDT
Created attachment 541844 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Review comments addressed.
Comment 32 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-24 15:58:35 PDT
Created attachment 541845 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset
Comment 33 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-24 18:04:09 PDT
Comment on attachment 541845 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

Thank you.
Comment 34 :Ehsan Akhgari (busy, don't ask for review please) 2011-06-27 13:31:24 PDT
http://hg.mozilla.org/mozilla-central/rev/58655e365c91
http://hg.mozilla.org/mozilla-central/rev/d7ed8936e9f8

Please file a new bug if you still see more performance regressions.  Thanks!
Comment 35 Ioana (away) 2011-09-02 06:13:17 PDT
Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Build ID: 20110831124126

To verify this fix I used the steps in comment #3. The response time is good now when using the Japanese IME.

Note You need to log in before you can comment on or make changes to this bug.