Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimizations #69

Open
jonschlinkert opened this issue May 12, 2016 · 3 comments
Open

optimizations #69

jonschlinkert opened this issue May 12, 2016 · 3 comments

Comments

@jonschlinkert
Copy link

I'd like to propose some optimizations that I would also be happy to implement after pr #67 is resolved.

In particular, I think if path caching, lazy-caching and getters/setters are implemented throughout the code it would result in pretty noticeable performance improvements. Just as importantly we can do so without complicating the code (it might even make the code cleaner).

Why?

  • lazy-caching: will result in faster initialization and application loading (potentially dramatically faster)
  • getters/setters: will result in better performance with path calculations and building up the config/env objects after initialization.
  • caching: will result in faster lookups in cases where the same information is requested more than once

Also, I implemented similar optimizations in verb and assemble and it reduced init time from 500-800 ms, to 150-200 ms. This makes the experience a lot nicer, and since init is faster it opens up the door for more interesting things (that might typically bog down the experience too much).

lazy-caching

Since liftoff is a command line application, it's a perfect use case for lazily evaluating requires. This way only libraries that are actually needed with be loaded upon invocation.

(fwiw, I've seen some developers have philosophical issues with this, but we use lazy-caching quite extensively and after 30+ million downloads of lazy-cache, we've had only one real issue related to webpack. And that was resolved months ago. Being a command line app, this would never be an issue here anyway)

Getters/setters

Anywhere an object is built up and paths are process, we have an opportunity to use getters/setters to evaluate and set the paths - similar to vinyl.

This way as the config/env is built up, whenever possible we would delay any path processing until it's needed.

Caching

This one probably wouldn't have much impact unless liftoff was used throughout the application lifecycle.
We would keep a cache of directories and files, and then whenever possible reuse the cached information instead of hitting the file system again.

@tkellen
Copy link

tkellen commented May 12, 2016

Huge 👍 to all of this @jonschlinkert. I look forward to discussing these updates in the future!

@phated
Copy link
Member

phated commented May 12, 2016

I think this is a good idea. However, it would need to be a breaking release and we need to heavily test it against gulp use cases. As we found out with a different module of @jonschlinkert's (I think it was some globbing module), people are bundling gulp up with webpack which didn't take kindly to the lazy caching.

@jonschlinkert
Copy link
Author

I agree, it would be a breaking release.

people are bundling gulp up with webpack which didn't take kindly to the lazy caching.

Lol yeah, that's the one issue I mentioned. lazy-cache was used in micromatch. Right after micromatch was merged into glob-stream an issue was created on glob-stream because of a bug in webpack (related to getters) - which surfaced because of lazy-cache's use of getters. "perception is reality", so it looked like a micromatch and/or lazy-cache bug (and it was a regression in glob-stream either way), so we removed lazy-cache just to get rid of the regression and perception. Within 24-48 hrs we also found a couple of different solutions to the webpack bug to prevent future issues (webpack still has the bug, but lazy-cache is no longer effected by it).

anyway I don't want to make lazy-cache a center point of this. I can just break this into in a few different prs so we can exclude the lazy-caching if you want. That part will take less than 5 minutes to add or remove, so we shouldn't lose sleep over it. I can just do it and we can compare before/after and discuss - or just not do it.

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

No branches or pull requests

3 participants