sindresorhus / atom-editorconfig

Helps developers maintain consistent coding styles between different editors
https://atom.io/packages/editorconfig
MIT License
811 stars 80 forks source link

Check if startup time can be improved #147

Closed florianb closed 1 year ago

florianb commented 7 years ago

Issuehunt badges

We're probably far slower than 5ms loading editorconfig.

IssueHunt Summary ### Sponsors (Total: $60.00) - [issuehunt issuehunt](https://issuehunt.io/u/issuehunt) ($60.00) #### [Become a sponsor now!](https://issuehunt.io/r/sindresorhus/atom-editorconfig/issues/147) #### [Or submit a pull request to get the deposits!](https://issuehunt.io/r/sindresorhus/atom-editorconfig/issues/147) ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/sindresorhus/atom-editorconfig/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.
Alhadis commented 7 years ago

@florianb What're you seeing? EditorConfig isn't even showing up for me in Timecop:

Figure 1
florianb commented 7 years ago

@Alhadis -- that's a good question. The last time i was looking editorconfig loaded with 41ms.

image

I do not take the benchmark too serious but i guess it's worth checking again if everything in this package is lazy enough to fit to its developers.. 🍸 🏖

Alhadis commented 7 years ago

Having editors open at startup increases activation time. Probably because the package is modifying the workspace before it's had a chance to finish initialising.

You can use onDidActivateInitialPackages to delay execution of the package's activation handler:

const activate = () => {
    atom.packages.onDidActivateInitialPackages(() => {
        generateConfig();
        showState();
        fixFile();
        atom.workspace.observeTextEditors(observeTextEditor);
        atom.workspace.observeActivePaneItem(observeActivePaneItem);
    });
};
florianb commented 7 years ago

Thanks @Alhadis 🦄 🔥 , this looks promising.. i will check that as soon i have some spare time! 👏

florianb commented 7 years ago

Thanks again @Alhadis - this will come with the next minor release. 🎉

florianb commented 7 years ago

Even though changing the intializing call chain is the right thing to do it breaks all unit tests with the two following results:

1.) ..."Error deactivating package 'editorconfig'", source: /Applications/Atom.app/Contents/Resources/app.asar/src/package.js (679) - i couldn't figure out what this means, yet. 2.) The necessary textEditor-instances of the tests aren't set anymore, since the waitsForPromise-statement before each test gets a) the results of atom.packages.activatePackage('editoroconfig') too early and b) it seems like the results do not contain the opened editor any more (see f.e. base-spec#16).

It seems i have to spend more time on this.

flexbox commented 7 years ago

Hello there! Thank you for all you package is aweome!

But for me the loadingtime jump to 800+ ms screen shot 2017-06-12 at 14 19 52

Any clue about that?

florianb commented 7 years ago

Hi @flexbox - thank you for getting in touch and thanks for the flowers.

Such a huge startup-time should definitely not be normal. However, we know about an issue happening when (re)opening a project with a lot of files at once: #191

Does your issue belong to that issue, too?

flexbox commented 7 years ago

repo/api 42360 files editorconfig --- 809ms

repo/personnal-website 483 files editorconfig --- 18ms

Smart investigation dear Watson, I don't know if you are watching node_modules/, bower_components/ and others folders like tmp/ but maybe you could avoid it

florianb commented 7 years ago

Do you have any Files opened in TextEditor-Panes when starting Atom? 🤔 Editorconfig do much without a referencing, opened Buffer/TextEditor.

flexbox commented 7 years ago

5 files max — It's not a big deal because I am using both sublime text and atom for differents things

Arcanemagus commented 7 years ago

@florianb Just to note, as I said in #191 the number of files open doesn't matter, only whether there is a TextEditor with a file open.

Also @flexbox either you need to update linter-eslint, or it's your only linter provider you have installed and is unfortunately getting saddled with a large chunk of the initialization time of linter itself.

florianb commented 7 years ago

Thank you @Arcanemagus i just hadn't had the time to dig through your entire post, yet.

@flexbox thank you - i will address that issue soon, as soon as possible.

IssueHuntBot commented 5 years ago

@issuehunt has funded $60.00 to this issue.


sindresorhus commented 1 year ago

Closing as Atom is abandoned.