improve startup time

[dirk-thomas]

The line in activate (require('atom-package-deps').install()) is responsible for the major part of the remaining package activation time.

@Arcanemagus @steelbrain Do you have any feedback on how this can be improved further? Also any hint about what the commonly applied pattern is would help me to decide what to do here and other packages.

[steelbrain]

While this should improve startup speed, a method I follow to make sure my packages are fast at startup is split the main and index file, so the index file looks something like

'use babel'
module.exports = {
  config: {... config here ...},
  instance: null,
  activate() {
    this.instance = new (require('./main'))()
  },
  deactivate() {
    this.instance.dispose()
  }
}

Because the requires and anything else is completely isolated in the main file, the loading is fast. This way we don't have to try to optimize requires in the main file so it can still be beautiful.

Also I noticed that you are requiring modules each time a function is executed, you might wanna change it to

helpers = null
module.exports  =
  activate: ->
  provideLinter: ->
    helpers ?= require('atom-linter')

This way the require from one method will be cached and methods that are executed often won't cost super much like they do now

[dirk-thomas]

The current state of the PR is indeed not ready to be merged. It was more a test how low I can get the activation time.

I understand that splitting index and main reduced the load time of the package. But if the main file still has all the requires at the time the activation time will still be high, correct? So I would conclude that the changes from this PR as well as the principle from your the second example to call require only once should definitely be applied - even when index and main are split. Do you agree with this?

Now back to the activate function: using atom-package-deps seems to be a significant overhead considering that it only benefits the case where users didn't install the dependencies correctly. Without the activation time is below 5ms - with it the time is around 20-30ms for me. While it was recently added to make it more convenient and "safer" if I look at the overhead and potentially dozens of packages doing that I am not sure if it is worth the overhead?

[steelbrain]

There is a Activation Time of a package, and Loading Time of a package, if you follow example 1, I'll make sure your package loading is instant and will move that time to package activation instead.

If you want to achieve superior results, yes, using both would give you superior results. About atom-package-deps, I am not really sure where that time is taken because all it does is, predict the name of package, read it's manifest and check if those packages are installed.

If you want atom-package-deps to perform faster, you can feed in your package name as the first parameter so it won't have to figure it out by creating error objects and running regexes over them. Can you try that and tell me if it makes a significant difference?

[dirk-thomas]

I used the package linter-pyflakes to test this since it contains much less code that linter-xmllint. TimeCop lists the following activation time for these calls in the activate functions:

• require('atom-package-deps').install(): 30ms
• require('atom-package-deps').install('linter-pyflakes'): 12ms

[dirk-thomas]

Even without splitting the files up I only see a high activation time for these packages. Can it be that Atom 1.4.x already delays loading non-crucial packages to activation time (according to what TimeCop reports)?

[steelbrain]

What version of atom-package-deps is that? We recently made regexes a lot more efficient, can you try it on the latest version and see how much faster it is?

[dirk-thomas]

The times are measured with the latest version of atom-package-deps (3.0.7). I did measure it in development mode though if that makes a difference?

(Btw. the repo https://github.com/steelbrain/package-deps doesn't contain a tag for 3.0.7?)

[steelbrain]

Would you mind opening a tracking bug in that repo about performance? About tags, I don't usually do them. The last time @Arcanemagus did, I guess I should too

[dirk-thomas]

I tried the latest version of atom-package-deps (3.0.8) but since the problem is still the same have filled steelbrain/package-/deps#38

[dirk-thomas]

I update this PR based on the feedback. While I can merge it I would appreciate any review and comment. Thanks.

[steelbrain]

This LGTM and should work

[Arcanemagus]

LGTM, thanks for taking the time to look into this!