halohalospecial / atom-elmjutsu

A bag of tricks for developing with Elm. (Atom package)
https://atom.io/packages/elmjutsu
MIT License
192 stars 24 forks source link

Elmjutsu takes too long to activate #76

Open pablomayobre opened 7 years ago

pablomayobre commented 7 years ago

Currently elmjutsu is the package, in my setup, that takes the longest time to activate, at 14 seconds! which is a lot considering this happens each time you open a window.

So I think that this package needs to consider improving this aspect of it's code (since the package is already amazing and pretty much stable). I want to start a discussion of possible improvements, noting that this is not a breaking issue.

This package can be considered a combination of multiple packages in one (autocomplete provider, goto, snippets, sidekick, install package, hyperclick, eval, repl). Can some of this functionality be split into different packages? Which parts rely on the same code?

To me I find the autocomplete and snippets to be the most useful part of the package, I don't commonly use the sidekick nor hyperclick nor eval/repl... Goto may be useful but I don't use it that often either.

halohalospecial commented 7 years ago

Hi, @Positive07!

Most of the features are inside classes (see the activate() function in https://github.com/halohalospecial/atom-elmjutsu/blob/master/lib/main.js). What we can do is to time the constructor() functions of those classes to see which are taking a lot of time. I'm all ears if you have suggestions on how to organize the code better (my JavaScript know-how is a bit outdated) :smiley:

How did you time package activation? From Time Cop? I agree that 14 seconds is a long time. Can you try running Atom in dev mode (atom -d) to check if elmjutsu is parsing files while activating?

Splitting elmjutsu into different packages may not bring a lot of benefits. I even considered integrating linter-elm-make with elmjutsu before so that only one elm-make process will be run for inferring types and getting the error messages :smiley: From my experience, people also get confused when functionality is split into different packages (https://twitter.com/todd1251/status/824846026422919171) and there may be more overhead in activating many packages. As of now, the user can decide which features to enable from the package settings.

halohalospecial commented 7 years ago

Oh sorry, are you suggesting something like https://nuclide.io?

pablomayobre commented 7 years ago

The advantage of splitting the package would be to only install those packages that you really need. But I can see what you mean about running a single elm-make process.

I used Time Cop to time the package activation. I can probably make some more tests and post the results (I'll do what you proposed and time the constructors to see what is the bottleneck or how the load is distributed).

It seems that Atom performs some optimizations after running a couple times, and it goes down to 9 seconds, and if you open a project which has no elm files open this can be further reduced to 6/7 seconds. When you have an Elm project open and you lunch Atom in dev-mode Elmjutsu prints a lot of parsing and readings for each documentation file.

Also it must be noted that Elmjutsu traverses the path up until it finds an elm-package.json which may not actually belong to the current project. In addition to searching for the elm-package.json elmjutsu should check that at least one of the src paths matches (or is a child of) the current project otherwise there is no need to compile the elm files

Thanks for the interest!

halohalospecial commented 7 years ago

@Positive07, thanks for your help! I didn't realize that elmjutsu is contributing that much to the lengthy activation time in Atom :smiley: We can probably load some stuff lazily (like the Sidekick panel).

You might also be interested in the upcoming Atom 1.17.

I created a new issue for this so that we can track it separately:

Also it must be noted that Elmjutsu traverses the path up until it finds an elm-package.json which may not actually belong to the current project. In addition to searching for the elm-package.json elmjutsu should check that at least one of the src paths matches (or is a child of) the current project otherwise there is no need to compile the elm files.

pablomayobre commented 7 years ago

It looks like better-queue calculates an UUID for each task you perform on a Queue, the UUID use a pseudo-random number generator function using various math operations in JavaScript which is not really efficient and takes some time to load.

The UUID is not really necessary a number would just do, this improvement has already been pointed out in the issue #12 in the better-queue repo.

Maybe this is a good start point to start improving on. I don't have actual numbers but I'll keep on investigating and reporting here.

halohalospecial commented 6 years ago

@Positive07, FWIW, I removed better-queue and used atom-linter's uniqueKey property instead.