Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Improve Atom startup time #276

Merged
merged 3 commits into from
Nov 18, 2016

Conversation

walles
Copy link
Contributor

@walles walles commented Nov 18, 2016

Before this change, activation was done on Atom startup, whether or not
you actually had any Python files open.

With this change in place, we postpone activation until the Atom Python
grammar's first use.

This improves startup time of my Atom by about 100ms, fixing one of the
top startup time offenders according to TimeCop.

For some frame of reference, I have 90 packages installed, and Timecop
lists all packages with startup times above 5ms.

Before this change, activation was done on Atom startup, whether or not
you actually had any Python files open.

With this change in place, we postpone activation until the Atom Python
grammar's first use.

This improves startup time of my Atom by about 100ms, fixing one of the
top startup time offenders according to TimeCop.

For some frame of reference, I have 90 packages installed, and Timecop
lists all packages with startup times above 5ms.
@walles
Copy link
Contributor Author

walles commented Nov 18, 2016

Note that when running the specs locally, it fixes the range for certain errors failed both with and without this patch applied.

@Arcanemagus
Copy link
Member

Unfortunately to properly test locally you need the hack module installed, which is very out of date as far as every other module, so I would completely expect that to fail locally 😞.

atom.packages.activatePackage('language-python'));

waitsForPromise(() =>
atom.workspace.open(goodPath));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move this as a .then on the above Promise.

@@ -12,14 +12,22 @@ describe('The flake8 provider for Linter', () => {
const lint = require('../lib/main.js').provideLinter().lint;

beforeEach(() => {
// This whole beforeEach function is inspired by:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I want this on every repo, maybe just update AtomLinter/Meta#15 with a link to that discussion?

@Arcanemagus
Copy link
Member

Arcanemagus commented Nov 18, 2016

Also, sorry I hadn't helped out on the js-yaml repo with this, been busy catching up in other repos (like this one 😛).

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there 😉.

)
);
atom.packages.activatePackage('language-python').then(() =>
waitsForPromise(() =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The waitsForPromise on L20 is already waiting on the parent of this, just return the open(), eg:

waitsForPromise(() =>
  atom.packages.activatePackage('language-python').then(() =>
    atom.workspace.open(goodPath)
  )
);

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just waiting on CI now.

@Arcanemagus Arcanemagus merged commit 84d70dc into AtomLinter:master Nov 18, 2016
@walles walles deleted the walles/fix-startup-time branch November 18, 2016 21:46
@hvdklauw
Copy link
Contributor

Break if you use a different python syntax like Python Django or MagicPython: #280

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants