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

RFC: Performance Proposal #110

Open
gdborton opened this issue Jul 25, 2017 · 10 comments
Open

RFC: Performance Proposal #110

gdborton opened this issue Jul 25, 2017 · 10 comments

Comments

@gdborton
Copy link
Contributor

I've been running lots of profiles to find the bottlenecks in our webpack build. Probably the biggest bottleneck right now is resolving requests.

To highlight this, I've created a simple test case for our build. Basically I intercepted all calls to Resolver.resolve during a build, and wrote the parameters passed to disk. Then I created a script that loaded the list of requests and used enhanced-resolve to resolve each of the requests, and also used resolve-from.

The results are pretty crazy, for 14850 requests:

  • enhanced-resolve: 11601 ms
  • resolve-from: 484 ms

Note that I didn't do much investigation into where this time is going, but I think that this project (and webpack) would benefit greatly from simplifying the architecture.

I'd prefer to see this library only respect an extensions option, and then stuff like custom description files (like bower.json or component.json) could be added via a loader (whose results could be cached). We could add support for browser fields and aliases the same way (cacheable loader).

@gdborton
Copy link
Contributor Author

@sokra @mikesherov @TheLarkInn I'd love to unlock the performance potential here, but recognize that my proposed changes would mean a breaking change in both enhanced-resolve and webpack.

This is what I envision the config would look like after this change.

module.exports = {
  entry: 'somefile.js',
  output: {
    path: path.resolve('./dist'),
    filename: 'outputname.js',
  },
  module: {
    use: [
      // These loaders would transform the require statements, so that
      // enhanced-resolve only sees what is intended to be imported.
      {
        loader: 'bower-library-loader',
        include: ['./src']
      },
      {
        loader: 'alias-loader',
        aliases: {
          jquery: path.resolve('./bower_components/jquery'),
          path.resolve('./src/someFile'): path.resolve('./src/someOtherFile'),
        },
      },
    ],
  },
}

@gdborton
Copy link
Contributor Author

gdborton commented Aug 8, 2017

@sokra poke.

@mikesherov
Copy link
Contributor

@gdborton, I think this is super interesting, but I was wondering if a different approach can work, where we introduce an option to use native resolution. That way, for most modules, you'd get the fast path of native resolution, with fallback to the other important resolvers (extensions comes to mind as an important one for React / Vue / Typescript) and other fast resolvers like the unsafeCachePlugin.

Thoughts?

@gdborton
Copy link
Contributor Author

gdborton commented Aug 8, 2017

I'm guessing that would work for most people outside of aliasing and those still using bower. Native resolving works for any file so long as you provide the extension in your import.

@mikesherov
Copy link
Contributor

Right but the key would fallback to slow manual resolution if fast resolution fails to find a module. So best of both worlds. You just wouldn't be able to do things like prioritize .vue over .js, etc.

@mikesherov
Copy link
Contributor

so aliases are interesting in that some folks use them to override stuff that would otherwise still resolve.

@mikesherov
Copy link
Contributor

So there's a tradeoff for sure. But I guess as a first step, you can inspect passed in options, and see if they match nodes native behavior, and if so, just go straight to native resolution.

@gdborton
Copy link
Contributor Author

gdborton commented Aug 8, 2017

Do you think that this should be a change in webpack without looking at enhanced-resolve at all?

Something like...

try {
  // resolve-from
  resolveFrom(/* options */)
} catch(e) {
  // enhanced-resolve
  Resolver.resolve(/* options */);
}

Or would this take place in enhanced-resolve and would be transparent to webpack?

I'm guessing the latter...

@mikesherov
Copy link
Contributor

I'm guessing latter, especially since it's a resolve option, which are automatically passed to a resolver factory.

@blowery
Copy link

blowery commented Jul 22, 2018

We're doing some profiling of the build for https://github.com/Automattic/wp-calypso and finding similar results; we're spending a lot of time resolving requests. However, we also use aliases heavily...

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