search

deech / fltkhs

Haskell bindings to FLTK GUI toolkit.
MIT License
190 stars 24 forks source link

addAndGetMenuItem is suprisingly slow #166

Open ericu opened 4 years ago

ericu commented 4 years ago

I was profiling to figure out why an operation in my code was slow, and it turned out that it wasn't at all--it was that I was updating a few menus with the result of the operation. Apparently down inside addAndGetMenuItem in my app there are 32454 calls to getMenuItemByIndex'. I do have a lot of stuff in my menus [I generate them based on what's in my database, so that you can select from any defined item]; it looks like I have somewhere north of 8000 of them, though far less than 32K. Here I'm updating a submenu with about a dozen entries by clearing it, putting back the standard items, and adding a new one to represent a newly-created object. What's the fast way to do this, or can addAndGetMenuItem be sped up?

The reason I'm using addAndGetMenuItem is to have a handle to the menu item [a toggle button] so that I can call "set" on it if it's the active item. If I could just add it with some flag that makes it set, I'd do that instead, but I don't see a flag for that. Perhaps I'm missing something?

ericu commented 4 years ago 
         addAndGetMenuItem                                                       Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:2700:164-293             65390          3    0.0    0.0    66.2   88.5
          dispatch                                                               Graphics.UI.FLTK.LowLevel.Dispatch                  src/Graphics/UI/FLTK/LowLevel/Dispatch.hs:151:1-83                  65396         12    0.0    0.0    66.2   88.5
           castTo                                                                Graphics.UI.FLTK.LowLevel.Dispatch                  src/Graphics/UI/FLTK/LowLevel/Dispatch.hs:134:1-24                  65398         12    0.0    0.0     0.0    0.0
           runOp                                                                 Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:243:3-63            65460          3    0.0    0.0     0.0    0.0
            withRef                                                              Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(467,1)-(473,8)          65461          3    0.0    0.0     0.0    0.0
             throwStackOnError                                                   Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(446,1)-(450,47)         65462          3    0.0    0.0     0.0    0.0
           runOp                                                                 Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(144,3)-(156,29)    65449          3    0.0    0.0     0.0    0.0
            withRef                                                              Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(467,1)-(473,8)          65450          3    0.0    0.0     0.0    0.0
             throwStackOnError                                                   Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(446,1)-(450,47)         65451          3    0.0    0.0     0.0    0.0
           runOp                                                                 Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(229,3)-(230,91)    65405          3    0.0    0.0     0.0    0.0
            addMenuItem                                                          Graphics.UI.FLTK.LowLevel.Base.MenuItem             src/Graphics/UI/FLTK/LowLevel/Base/MenuItem.chs:(287,1)-(328,29)    65406          3    0.0    0.0     0.0    0.0
             addMenuItem.\                                                       Graphics.UI.FLTK.LowLevel.Base.MenuItem             src/Graphics/UI/FLTK/LowLevel/Base/MenuItem.chs:(289,18)-(290,73)   65407          3    0.0    0.0     0.0    0.0
              withRef                                                            Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(467,1)-(473,8)          65408          3    0.0    0.0     0.0    0.0
               throwStackOnError                                                 Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(446,1)-(450,47)         65409          3    0.0    0.0     0.0    0.0
             addMenuItem.go                                                      Graphics.UI.FLTK.LowLevel.Base.MenuItem             src/Graphics/UI/FLTK/LowLevel/Base/MenuItem.chs:(296,7)-(328,29)    65424          0    0.0    0.0     0.0    0.0
              addMenuItem.go.combinedFlags                                       Graphics.UI.FLTK.LowLevel.Base.MenuItem             src/Graphics/UI/FLTK/LowLevel/Base/MenuItem.chs:297:13-52           65438          0    0.0    0.0     0.0    0.0
               menuItemFlagsToInt                                                Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:387:1-74                     65439          0    0.0    0.0     0.0    0.0
                combine                                                          Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:159:1-52                     65440          0    0.0    0.0     0.0    0.0
                 fromEnum                                                        Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(263,3)-(272,35)         65441          3    0.0    0.0     0.0    0.0
              copyTextToCString                                                  Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:(441,1)-(444,30)             65425          0    0.0    0.0     0.0    0.0
               copyTextToCString.bs                                              Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:442:7-25                     65426          0    0.0    0.0     0.0    0.0
           runOp                                                                 Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(233,3)-(239,195)   65397          3    0.0    0.0    66.2   88.5
            add                                                                  Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:1978:106-207             65399          3    0.0    0.0     0.0    0.0
             withRef                                                             Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(467,1)-(473,8)          65400          3    0.0    0.0     0.0    0.0
              throwStackOnError                                                  Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(446,1)-(450,47)         65401          3    0.0    0.0     0.0    0.0
               withRef.\                                                         Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(470,25)-(472,29)        65402          3    0.0    0.0     0.0    0.0
                add.\                                                            Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:1978:156-164             65404          3    0.0    0.0     0.0    0.0
                toRefPtr                                                         Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(460,1)-(464,21)         65403          3    0.0    0.0     0.0    0.0
            getMenu                                                              Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:2680:122-231             65443          3    0.0    0.0     0.0    0.0
             withRef                                                             Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(467,1)-(473,8)          65444          3    0.0    0.0     0.0    0.0
              throwStackOnError                                                  Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(446,1)-(450,47)         65445          3    0.0    0.0     0.0    0.0
               withRef.\                                                         Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(470,25)-(472,29)        65446          3    0.0    0.0     0.0    0.0
                getMenu.\                                                        Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:2680:176-184             65448          3    0.0    0.0     0.0    0.0
                toRefPtr                                                         Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(460,1)-(464,21)         65447          3    0.0    0.0     0.0    0.0
            runOp.mi                                                             Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:236:9-23            65478          3    0.0    0.0     0.0    0.0
            runOp                                                                Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(144,3)-(156,29)    65452          0    0.0    0.0    66.2   88.5
             withRef                                                             Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(467,1)-(473,8)          65453          3    0.0    0.0    66.2   88.5
              throwStackOnError                                                  Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(446,1)-(450,47)         65454      32454    0.0    0.0    66.2   88.5
               withRef.\                                                         Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(470,25)-(472,29)        65455          9    0.0    0.0    66.2   88.5
                toRefPtr                                                         Graphics.UI.FLTK.LowLevel.Fl_Types                  src/Graphics/UI/FLTK/LowLevel/Fl_Types.chs:(460,1)-(464,21)         65456          9    0.0    0.0     0.0    0.0
                getSize.\                                                        Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:2612:176-184             65459          3    0.0    0.0     0.0    0.0
                runOp.\                                                          Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:243:50-63           65464          3    0.0    0.0     0.0    0.0
                 size'                                                           Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(242,1)-(246,15)    65465          3    0.0    0.0     0.0    0.0
                  size'.\                                                        Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(245,3)-(246,15)    65467          3    0.0    0.0     0.0    0.0
                   size'.\.res'                                                  Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:245:8-30            65468          3    0.0    0.0     0.0    0.0
                  size'.a1'                                                      Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:243:8-18            65466          3    0.0    0.0     0.0    0.0
                runOp.\                                                          Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(144,50)-(148,32)   65457          3    0.0    0.0    66.2   88.5
                 runOp.go                                                        Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(150,9)-(156,29)    65469      32454   66.1   88.3    66.2   88.5
                  getMenuItemByIndex'                                            Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(143,1)-(148,15)    65470      32451    0.0    0.0     0.0    0.0
                   getMenuItemByIndex'.\                                         Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:(147,3)-(148,15)    65473      32451    0.0    0.0     0.0    0.0
                    getMenuItemByIndex'.\.res'                                   Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:147:8-20            65475      32451    0.0    0.0     0.0    0.0
                   getMenuItemByIndex'.a1'                                       Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:144:8-18            65471      32451    0.0    0.0     0.0    0.0
                   getMenuItemByIndex'.a2'                                       Graphics.UI.FLTK.LowLevel.Base.MenuPrim             src/Graphics/UI/FLTK/LowLevel/Base/MenuPrim.chs:145:8-18            65472      32451    0.0    0.0     0.0    0.0
                  toMaybeRef                                                     Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:357:1-90                     65474      32451    0.0    0.0     0.1    0.1
                   toRef                                                         Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:(326,1)-(330,35)             65476      32451    0.0    0.0     0.1    0.1
                    wrapNonNull                                                  Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:(190,1)-(195,57)             65477      32451    0.1    0.1     0.1    0.1
                    toRef.result                                                 Graphics.UI.FLTK.LowLevel.Utils                     src/Graphics/UI/FLTK/LowLevel/Utils.hs:329:25-45                    65483          3    0.0    0.0     0.0    0.0
                 getSize                                                         Graphics.UI.FLTK.LowLevel.Hierarchy                 src/Graphics/UI/FLTK/LowLevel/Hierarchy.hs:2612:122-231             65458          3    0.0    0.0     0.0    0.0
                 runOp
ericu commented 4 years ago

It occurs to me that in the C FLTK the AtIndex is much more useful. You can call menu() to get the underlying array of menus, and then menu()[atIndex] should be the menu item you want to modify. In your wrapper, there's no way I see to do that, so I had to use this function that does a linear search.

Huh. Looking at the source of AddAndGetMenuItem really looks like it's doing what I'd want to do. I think it must be doing something per-menu-item that's not obvious from the code, which looks no more expensive than a list traversal:

instance (Parent a MenuItemBase, impl ~ ( T.Text -> Maybe Shortcut -> Maybe (Ref a-> IO ()) -> MenuItemFlags -> IO (Ref MenuItemBase))) => Op (AddAndGetMenuItem ()) MenuPrimBase orig (impl) where runOp menu name shortcut cb flags = do (AtIndex i) <- add menu name shortcut cb flags items <- getMenu menu_ let mi = items !! i case mi of Just mi' -> return mi' Nothing -> throwIO (userError ("FLTK claims the menu item " ++ (T.unpack name) ++ " was added successfully at index " ++ (show i) ++ " , but no MenuItem at that index could be retrieved."))

Perhaps it's forcing some conversion on each list item by iterating past them? Could it be lazier?

deech commented 4 years ago

Off the bat I see that getMenu is retrieving all the items in the menu and iterating through them to find the one you just added, so essentially it's the painter's algorithm.

Honestly I didn't see the menus being used this way so I went with a very naive approach. An incremental improvement might be to have getMenu return a Vector instead of a List.

ericu commented 4 years ago

Could AddAndGetMenuItem just call into getMenuItemByIndex' more directly, as getMenu is?

deech commented 4 years ago

Yes that might work well. I'll try to get to it as soon as I can but you are able to I'm happy to merge a PR.

ericu commented 4 years ago

Thanks! Exposing getMenuItemByIndex would also be great.

ericu commented 4 years ago

Heh; and in this case, the flag I was looking for was MenuItemValue, so I'm just going to call add instead of addAndGetMenuItem :grin:. So no rush on my account, but it does look likely to be a quick fix when you get around to it.

ericu commented 4 years ago

I'm also going to see about reducing my menu count by a factor of 80-100; there's a small change that would do it, and now that I know how many there are, it seems kind of ridiculous.