lehitoskin / ivy

Ivy, the Taggable Image Viewer
GNU General Public License v3.0
16 stars 3 forks source link

Refactor code for friendlier startup times #69

Open IonoclastBrigham opened 6 years ago

IonoclastBrigham commented 6 years ago

So currently, it looks like pretty much every GUI component in the app is allocated and statically initialized at startup whether it's ever used or not, because of how the code is written. This leads to poor startup times, and is especially wasteful when using ivy as a command line utility.

This can be ameliorated by instead of top-level define-ing everything for each module, export one or more functions that will create the required GUI objects on demand. Or a concrete subclasses that can easily be instantiated with all the desired default attributes.

lehitoskin commented 6 years ago

I agree that the CLI should start faster and not require GUI libraries to load. This has been in the back of my mind for a while, but I just never made it into an issue.

IonoclastBrigham commented 6 years ago

The following naïve command took ~half an hour on my 2.3 GHz Core i7 Mac with an SSD:

$ for img in (ivy -ea '<some-tag>'); ivy —set-rating 4 "$img"; end

And if we take a look at how many items it's working on...

$ ivy -ea '<some-tag>'|wc -l
     363

...it comes out to like 5 or 6 seconds per image. Clearly, this is way too long for a tag search, a few database upserts, and stuffing an XMP field into the files. It should be much faster to have used xargs, resulting in only 2 ivy invokations. But looping is still a usecase we should expect, especially if the particular sequence of actions doesn't naturally support spamming the command line with hundreds of arguments (e.g. renaming files in-place).

IonoclastBrigham commented 6 years ago

Using xargs to pass all the image paths in argv instead of doing one at a time in a loop, we get:

$ ivy -nea '<some-tag>' | xargs -0 time ivy --set-rating 4
       23.29 real        16.76 user         2.27 sys

This is about a 90X speedup. Clearly, avoiding multiple ivy invokations/init routines is a huge optimization.

lehitoskin commented 6 years ago

I started to play around with this and there's one thing I've noticed that immediately makes this a much larger issue: base.rkt needs to be pared down as small a footprint as possible. Only require the bare minimum libraries and nothing that is heavy, like racket/gui/base. This will require breaking out the image loading into its own file (which was on the docket already, but never formally tracked on GitHub.)

This is going to become a project, with this issue depending on the refactor of load-image before we can proceed.

IonoclastBrigham commented 6 years ago

I was thinking it might be nice to move load-image into ivy-canvas%.