TISCreateInputSourceList causes Main Thread IO (200ms after startup)

NEW
Unassigned

Status

()

7 years ago
5 years ago

People

(Reporter: BenWa, Unassigned)

Tracking

({main-thread-io})

Trunk
x86
macOS
main-thread-io
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sps])

Attachments

(1 attachment)

(Reporter)

Comment 1

7 years ago
Corrected to about 200ms by zooming out in the profile selection (not linked above)
Summary: TISCreateInputSourceList causes Main Thread IO (40ms after startup) → TISCreateInputSourceList causes Main Thread IO (200ms after startup)
Created attachment 634005 [details] [diff] [review]
Patch

How about this? This patch caches US keyboard layout for handling shortcut keys. The shortcut key handler is the only caller calling InitByInputSourceID() in normal usage. The other caller is API for automated tests.
Attachment #634005 - Flags: review?(smichaud)
(Reporter)

Comment 3

7 years ago
Any idea what to expect to call this for automated testing? We may automatically look for IO during testing.

Will this patch also remove IO for other non English local?
(In reply to Benoit Girard (:BenWa) from comment #3)
> Any idea what to expect to call this for automated testing? We may
> automatically look for IO during testing.

It's used for testing the native key event handling code in any layouts.

I don't think that we should optimize the testing performance with complicated code or wasting memory never used actually because the performance isn't so bad and the tests are not so many.

> Will this patch also remove IO for other non English local?

Yes. US keyboard layout is used for detecting some keyboard layouts which switch the keys when Cmd key is pressed like Dvorak-QWERTY. So, it's being used with any keyboard layouts.
Benoit, does Masayuki's patch actually stop TISCreateInputSourceList() being called on startup?  (TISCreateInputSourceList() must still be called once, and I don't know if Masayuki's patch changes when that call happens.)

And is it possible to extract a stack trace of the call(s) to TISCreateInputSourceList() from your profile (in comment #0)?
(Reporter)

Comment 6

7 years ago
(In reply to Steven Michaud from comment #5)
> Benoit, does Masayuki's patch actually stop TISCreateInputSourceList() being
> called on startup?  (TISCreateInputSourceList() must still be called once,
> and I don't know if Masayuki's patch changes when that call happens.)
> 
> And is it possible to extract a stack trace of the call(s) to
> TISCreateInputSourceList() from your profile (in comment #0)?

I've been wanting this as well. Bug 765813
Actually it took me a while even to *find* the call to TISCreateInputSourceList().  I ended up setting the "filter" to "TISCreateInputSourceList", then laboriously expanding each individual level in the stack trace.  (So Cleopatra should also support "expand all".)

What I found puzzles me:  TISCreateInputSourceList() is called (indirectly) from [nsChildView keyDown:] -- in other words on processing a native keyDown event.  This isn't something you'd normally expect to happen on startup.

Do you have any idea, Benoit, why it happened while you were profiling?

Could you try profiling again, and this time make sure you don't hit any keyboard keys while it's happening?
> [nsChildView keyDown:]

[ChildView keyDown:]
Benoit: Could you check comment 7?
Flags: needinfo?(bgirard)
(Reporter)

Comment 10

6 years ago
Alright so the problem still occurs but I only see the execution happening when I press a key which I often do to save a profile.
Flags: needinfo?(bgirard)
> I only see the execution happening when I press a key which I often
> do to save a profile

You're doing this on startup?

I still don't understand.
(Reporter)

Comment 12

6 years ago
(In reply to Steven Michaud from comment #11)
> > I only see the execution happening when I press a key which I often
> > do to save a profile
> 
> You're doing this on startup?
> 
> I still don't understand.

This shows up when you do your first keypress. My startup profiling workflow caught it because I would restart the browser and press APPLE+SHIFT+O to open the profile.
Comment on attachment 634005 [details] [diff] [review]
Patch

Does this still make sense?  Is this still wanted?
(Reporter)

Comment 14

5 years ago
We still want a solution to this problem. I don't know enough to review this patch.
I guess that the patch still works. I'll confirm it ASAP.
You need to log in before you can comment on or make changes to this bug.