Using Japanese IME, response of input is painfully slow on Wikipedia editing page

VERIFIED FIXED in mozilla7

Status

()

Core
Editor
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: Ehsan)

Tracking

({inputmethod, regression})

Trunk
mozilla7
x86
Windows 7
inputmethod, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 6 obsolete attachments)

4.53 KB, patch
Details | Diff | Splinter Review
3.61 KB, patch
masayuki
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
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
(Reporter)

Updated

6 years ago
Summary: Using Japanese IME, response of input is painfully slow. → Using Japanese IME, response of input is painfully slow on Wikipedia editing page
Assignee: nobody → ehsan
(Reporter)

Comment 1

6 years ago
*This happens at the time of the editing of the huge sentence and texts.

*And it does not happen if IME off.
I will profile this today.
(Reporter)

Comment 3

6 years ago
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
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)
(Note that I don't speak Japanase, so telling me exactly which keys to press would be most helpful) :-)
(Reporter)

Comment 6

6 years ago
(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).
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
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
(Reporter)

Comment 9

6 years ago
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
OK, I managed to reproduce this.  Thanks for your help.  I'll investigate.
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.
Created attachment 541185 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets
Attachment #541185 - Flags: review?(roc)
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.
Attachment #541199 - Flags: review?
Attachment #541199 - Flags: review? → review?(roc)
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 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]:
-----------------------------------------------------------------
Attachment #541185 - Flags: review?(roc) → review+
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.
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!
Attachment #541199 - Attachment is obsolete: true
Attachment #541235 - Flags: review?(roc)
Attachment #541199 - Flags: review?(roc)
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.
Attachment #541185 - Attachment is obsolete: true
Attachment #541251 - Flags: review?(roc)
Comment on attachment 541235 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

Review of attachment 541235 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #541235 - Flags: review?(roc) → review+
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]:
-----------------------------------------------------------------
Attachment #541251 - Flags: review?(roc) → review+
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.
Attachment #541251 - Attachment is obsolete: true
Attachment #541517 - Flags: review?(roc)
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.
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.  ;-)
Attachment #541235 - Attachment is obsolete: true
Comment on attachment 541521 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

...so are my bugzilla usage skills!
Attachment #541521 - Flags: review?(masayuki)
Attachment #541517 - Flags: review?(masayuki)
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.
Attachment #541517 - Flags: review?(masayuki) → review+
(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().
> Why don't you define name const or macro for 0xffffffff?

PR_UINT32_MAX?
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 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
Attachment #541517 - Flags: review?(roc) → review+

Comment 30

6 years ago
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
Created attachment 541844 [details] [diff] [review]
Part 1 - Optimize the conversion of native and cross platform text offsets

Review comments addressed.
Attachment #541517 - Attachment is obsolete: true
Created attachment 541845 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset
Attachment #541521 - Attachment is obsolete: true
Attachment #541845 - Flags: review?(masayuki)
Attachment #541521 - Flags: review?(masayuki)
Comment on attachment 541845 [details] [diff] [review]
Part 2: Optimize nsContentEventHandler::GetFlatTextOffset

Thank you.
Attachment #541845 - Flags: review?(masayuki) → review+
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!
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7

Comment 35

6 years ago
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.
Status: RESOLVED → VERIFIED
Depends on: 692145
You need to log in before you can comment on or make changes to this bug.