183072 - move IME stuff out of nsWindow (w32) into a structure and only allocate it if necessary

Status: RESOLVED FIXED

(Core :: Widget: Win32, enhancement, P4)

Description

[timeless]

I was looking through nsWindow and found that IME has a *whole* lot of fields,
and as you can see the mIME* fields are all empty for Western users (well,
except for mIMECursorPosition, i have no idea what's up with that).
	mIMEIsComposing	0 ''
	mIMEIsStatusChanged	0 ''
	mIMEProperty	0
+	mIMECompString	0x00000000
+	mIMECompUnicode	0x00000000
+	mIMEAttributeString	0x00000000 ""
	mIMEAttributeStringLength	0
	mIMEAttributeStringSize	0
+	mIMECompClauseString	0x00000000
	mIMECompClauseStringLength	0
	mIMECompClauseStringSize	0
	mIMECursorPosition	-842150451
+	mIMEReconvertUnicode	0x00000000
+	mIMECompCharPos	0x00000000 {top=??? bottom=??? left=??? right=???}
	uWM_MSIME_RECONVERT	49361
	uWM_MSIME_MOUSE	49363
	uWM_ATOK_RECONVERT	49362

Comment 1

[Alec Flett]

do we have an idea how big the overhead of the IME stuff is? can we get a
sizeof() summary? Remember that allocating an extra field has an overhead of 4
bytes (for the pointer to the storage on the heap) plus 8 bytes (for the
management of the new space on the heap) plus rounding up to the nearest 16 bytes.

So generally unless a structure is > 16 bytes, its not worth the overhead of
lazily creating it.

Comment 2

[timeless]

Attachment: draft

Comment 3

[timeless]

Attachment: checkpoint draft v.2

ok. this seems to work w/ MSIME 2000 Japanese.
it seems like we should convert from nsString* to nsAutoString, so i'll do that
in round three. but we could stop here.

Comment 4

[Alec Flett]

why can't you inline the nsString/nsCString members, rather than keeping
pointers to them? You should be able to use IsEmpty() (or even IsVoid()) instead
of null-checking.

Again, remember that the overhead of a pointed-to object is 4 bytes plus about 8
bytes for heap management for the new object. In the case of strings, which have
12 bytes of overhead, the cost is the same to make it inline as it is to store a
pointer.. but the inline version does memory management for you (in the
constructor/destructor) and fragments the heap less.. 

what's the sizeof(nsWinIME)? How many do we create when one window is open? when
5 are open?

Comment 5

[John Keiser (jkeiser)]

fyi, I've been running with this patch off and on for a couple of weeks and it
hasn't caused problems.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

timeless:

I changed the IME variables to static variables.(see bug 253035)
Even it, do you need to fix this bug?

Comment 7

[timeless]

i'd still rather not pay for the ime if it isn't used, but it certainly isn't a
top priority. i'll look at this again when the tree for it resurrects itself
(should happen soon).

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

taking.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

fixed by bug 487016.

-> FIXED