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

fix: Memory leak in watch mode and use Set for performance #130

Merged
merged 1 commit into from
Sep 29, 2017

Conversation

mikesherov
Copy link
Contributor

@mikesherov mikesherov commented Jun 17, 2017

TLDR

Before: 200ms per rebuild (and an additional 200ms for each additional rebuild)
After: ~5ms per rebuild!

The Long Version

Hi! Thanks for the awesome plugin! It has saved my life on many projects. One of my coworkers today was experiencing slow rebuilds in watch mode, and with a little bit of profiling, it turns out this plugin was the culprit. There are 2 issues:

  1. a memory leak that causes the fileDependencies and contextDependencies to grow after each save.
  2. An O(n) _.includes call that can be replaced with an O(1) Set.prototype.has call.

Here are the performance characteristics of this patch. The project in question is private, but has 1800 files. The did it line is how long this plugin takes to complete:

Before Fixing Memory Leak

did it: 386.245ms                                                     
Hash: fd31795dd4d692c70ccd
Version: webpack 2.3.3
Time: 2522ms

did it: 597.102ms                                                     
Hash: 77299979c1a9afc21683
Version: webpack 2.3.3
Time: 2863ms

did it: 827.018ms                                                     
Hash: fd31795dd4d692c70ccd
Version: webpack 2.3.3
Time: 2959ms

did it: 1078.640ms                                                    
Hash: 77299979c1a9afc21683
Version: webpack 2.3.3
Time: 2808ms

did it: 1298.503ms                                                    
Hash: fd31795dd4d692c70ccd
Version: webpack 2.3.3
Time: 3032ms

After Fixing Memory Leak

did it: 252.222ms                                                     
Hash: fd31795dd4d692c70ccd
Version: webpack 2.3.3
Time: 2157ms

did it: 220.383ms                                                     
Hash: 77299979c1a9afc21683
Version: webpack 2.3.3
Time: 1999ms

did it: 236.641ms                                                     
Hash: fd31795dd4d692c70ccd
Version: webpack 2.3.3

After Fixing Memory Leak + Switching to Set()

did it: 3.965ms                                                       
Hash: 77299979c1a9afc21683
Version: webpack 2.3.3
Time: 1949ms

did it: 2.391ms                                                       
Hash: fd31795dd4d692c70ccd
Version: webpack 2.3.3
Time: 1850ms

Let me know if you have any questions. Thanks!

@jsf-clabot
Copy link

jsf-clabot commented Sep 29, 2017

CLA assistant check
All committers have signed the CLA.

@joshwiens joshwiens changed the title Address memory leak in watch mode and use Set for performance fix: Memory leak in watch mode and use Set for performance Sep 29, 2017
Copy link
Member

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

5 participants