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

defer activating the package until the window is loaded. #485

Merged
merged 1 commit into from
Feb 15, 2019

Conversation

akonwi
Copy link
Contributor

@akonwi akonwi commented Feb 14, 2019

Using the latest version of atom (1.34.0 at this time), this package is consistently the slowest to activate. I've recorded the average activation time for this package in two scenarios;

  1. A window with one project open and reloaded 4 times: average activation time is 321ms
  2. A window with no project open and reloaded 4 times: average activation time is 222ms

Using this activation hook, atom will defer activating this package until the window is ready to use and then there's no impact to startup time. Since this package doesn't need to do heavy setup, I think it should wait as long as possible to activate anyway.

@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #485 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #485   +/-   ##
=======================================
  Coverage   75.91%   75.91%           
=======================================
  Files          27       27           
  Lines         436      436           
  Branches       42       42           
=======================================
  Hits          331      331           
  Misses         89       89           
  Partials       16       16

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f324f1d...fd658ef. Read the comment docs.

Copy link
Collaborator

@robwise robwise left a comment

Choose a reason for hiding this comment

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

Nice, this solves a recurring issue we've had where we keep getting startup performance regressions from new features being added.

Can you please rename the commit to use the conventional changelog standard, something like perf(startup): defer activating the package until the window is loaded?

@akonwi
Copy link
Contributor Author

akonwi commented Feb 15, 2019

Woops. Sorry, I skimmed the contributing guide and missed that part. I've updated the commit

@robwise
Copy link
Collaborator

robwise commented Feb 15, 2019

No worries, thanks so much for the PR :)

@robwise robwise merged commit 0e1c7b9 into prettier:master Feb 15, 2019
@akonwi
Copy link
Contributor Author

akonwi commented Feb 15, 2019

I just noticed #330. I guess this resolves that as well. After reading the suggestions there and looking at the code, I'm pretty confident that all the lazy* functions can be removed because they don't gain much when the package is activated outside of the initial window startup, which is so messy and suboptimal. I'll test and may open another PR

lgeiger added a commit to atom-community/ide-python that referenced this pull request Feb 15, 2019
This PR defers package activation until the shell environment is loaded. This should speed up startup time of Atom and doesn't impact `ide-python` since we need to wait for the shell environment to be loaded anyway.

See prettier/prettier-atom#485
lgeiger added a commit to atom-community/ide-python that referenced this pull request Feb 15, 2019
This PR defers package activation until the shell environment is loaded. This should speed up startup time of Atom and doesn't impact `ide-python` since we need to wait for the shell environment to be loaded anyway.

See prettier/prettier-atom#485
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants