Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Add Hard source Webpack Plugin for perf improvement #646

Merged
merged 11 commits into from
Sep 1, 2016
Merged

Conversation

thangngoc89
Copy link
Contributor

@thangngoc89 thangngoc89 commented Aug 22, 2016

There are 2 things that concern me:

  • the files and directories field must be config explicitly for each project. Checkout docs.
  • This doesn't work on node build (which is pretty fast by the way, around 4-5 seconds)

If we move this to user's land, we might need to merge this PR: #642 for determine whether this is a node build or a browser one.

Just found a bug, put this in WIP mode:

If I perform a static build and then run the dev server after that, there will be a additional css file which is inserted to that page as a script tag, this broke the page.

So we need to clean cache between static and dev mode.

<script src="/phenomic.browser.53c111cc11761331ee4b.js"></script>,<script src="/phenomic.browser.53c111cc11761331ee4b.css"></script>

@thangngoc89
Copy link
Contributor Author

I forgot about the benchmark:

khoa:~/web/phenomic/docs (hard-source-wp)$ time npm run build

> Phenomic@ build /home/khoa/web/phenomic/docs
> phenomic build

  phenomic:builder ✓ Static build: assets copy completed +0ms
Environment has changed (node_modules or configuration was updated).
HardSourceWebpackPlugin will reset the cache and store a fresh one.
  phenomic:builder ✓ Static build: client build completed +23s
  phenomic:builder ✓ Static build: static build completed +4s
  phenomic:static:to-html No json in for url 'showcase'. +254ms
  phenomic:static:to-html No json in for url 'showcase/tag/blog'. +73ms
  phenomic:static:to-html No json in for url 'showcase/tag/community'. +30ms
  phenomic:static:to-html No json in for url 'showcase/tag/docs'. +28ms
  phenomic:static:to-html No json in for url 'showcase/tag/event'. +29ms
  phenomic:static:to-html No json in for url 'showcase/tag/learning'. +19ms
  phenomic:static:to-html No json in for url 'showcase/tag/multi-languages'. +22ms
  phenomic:static:to-html No json in for url 'showcase/tag/open-source'. +22ms
  phenomic:static:to-html No json in for url 'showcase/tag/service'. +25ms
  phenomic:static ✓ Static html files: 51 files written. +832ms
  phenomic:static ✓ CNAME is phenomic.io. +0ms
  phenomic:static ✓ .nojekyll created. +1ms
  phenomic:static ✓ Build successful. +0ms

real    0m31.012s
user    0m31.664s
sys 0m1.144s
khoa:~/web/phenomic/docs (hard-source-wp)$ time npm run build

> Phenomic@ build /home/khoa/web/phenomic/docs
> phenomic build

  phenomic:builder ✓ Static build: assets copy completed +0ms
  phenomic:builder ✓ Static build: client build completed +10s
  phenomic:builder ✓ Static build: static build completed +5s
  phenomic:static:to-html No json in for url 'showcase'. +210ms
  phenomic:static:to-html No json in for url 'showcase/tag/blog'. +74ms
  phenomic:static:to-html No json in for url 'showcase/tag/community'. +17ms
  phenomic:static:to-html No json in for url 'showcase/tag/docs'. +14ms
  phenomic:static:to-html No json in for url 'showcase/tag/event'. +19ms
  phenomic:static:to-html No json in for url 'showcase/tag/learning'. +17ms
  phenomic:static:to-html No json in for url 'showcase/tag/multi-languages'. +16ms
  phenomic:static:to-html No json in for url 'showcase/tag/open-source'. +14ms
  phenomic:static:to-html No json in for url 'showcase/tag/service'. +16ms
  phenomic:static ✓ Static html files: 51 files written. +526ms
  phenomic:static ✓ .nojekyll created. +1ms
  phenomic:static ✓ CNAME is phenomic.io. +0ms
  phenomic:static ✓ Build successful. +1ms

real    0m19.670s
user    0m19.772s
sys 0m0.800s

@thangngoc89 thangngoc89 changed the title Add Hard source Webpack Plugin for perf improvement [WIP] Add Hard source Webpack Plugin for perf improvement Aug 22, 2016
@MoOx
Copy link
Owner

MoOx commented Aug 22, 2016

So we need to clean cache between static and dev mode.

This sucks so much. We need to report this, it definitely looks like a bug of this plugin.

@thangngoc89
Copy link
Contributor Author

Actually, this is my fault. HardSouceWebpackPlugin cache must be cleaned if the webpack config is changed. Our dev and static config is different so we'll need to use a different cache folder for dev and static

@thangngoc89
Copy link
Contributor Author

@MoOx looks good to you?

@thangngoc89
Copy link
Contributor Author

Appveyor hang at flow check step. Not relevant to this PR


const chunkNameBrowser = "phenomic.browser"

export default (config: PhenomicConfig): WebpackConfig => {
// Use different cache folder for hard-source-webpack-plugin on each env
const env = (config.production) ? "prod" : "dev"
Copy link
Owner

Choose a reason for hiding this comment

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

You should use a hash on the entire config object. Other flag might change and be used too like "config.static" or some others (user custom flags?). We need to be sure that we will have one cache per POSSIBLE config :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you prefer any hash algorithm?

Copy link
Owner

Choose a reason for hiding this comment

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

You can just use something like that https://www.npmjs.com/package/node-object-hash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Look like we are gonna store a lot of junk files in user's node modules folder.

Copy link
Owner

Choose a reason for hiding this comment

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

If we don't do that, we open ourselves to get a ton of issues for each people "specific use cases".

Copy link
Owner

Choose a reason for hiding this comment

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

I am pretty sure all efficient static gen have a lot of cache files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. Will store cach with hashed config object

@MoOx
Copy link
Owner

MoOx commented Aug 30, 2016

Any news?
This + dll will probably divide by 50% (or more) the build time. We need to ship this soon :)

@thangngoc89
Copy link
Contributor Author

@MoOx Will work on this soon. I'm trying to finish my client's website (phenomic based). IMHO, we don't need dll plugin once we have this. 20 secs for docs build is acceptable.

@MoOx
Copy link
Owner

MoOx commented Aug 30, 2016

dll plugins is definitely an important booster. 20s is too long. Go and ask Hugo users. :D

@MoOx
Copy link
Owner

MoOx commented Aug 30, 2016

dll plugins allows to only compile YOUR code (eg: compile a dependencies.dll.js with all npm deps, and only compile YOUR code in dev, which boost a LOT the build).

@MoOx
Copy link
Owner

MoOx commented Aug 30, 2016

I am using it at work and it's really nice, especially for a REALLY FAST hot loading.

@thangngoc89
Copy link
Contributor Author

Go and ask Hugo users. :D

I am a Hugo user. Lol. With go, build time is calculated with milisecond, not second

@MoOx
Copy link
Owner

MoOx commented Aug 30, 2016

I know, that's why I said that. But hugo users don't have an assets graphs like webpack offers :) Anyway if we can reduce time, we will.

@thangngoc89
Copy link
Contributor Author

I think we should ask Hard-source-webpack-plugin author's first before using DLL. If I'm not wrong hard source cache dependencies resolution too. And well, try this branch. reloading time is < 100ms

@MoOx
Copy link
Owner

MoOx commented Aug 30, 2016

Using dll is another step. dll is: you make a build of the deps you want, then you have a use dll.js + a manifest. Then for your other build, you consume the manifest. THis way webpack does not even have to compile the deps referenced. So I guess this is totally ok to use both projects.

@thangngoc89
Copy link
Contributor Author

@MoOx could you take a look ?

@thangngoc89 thangngoc89 changed the title [WIP] Add Hard source Webpack Plugin for perf improvement Add Hard source Webpack Plugin for perf improvement Aug 31, 2016
const hasher = new ObjectHash()
const hash = hasher.hash(config)

const cacheDir = findCacheDir({ name: "phenomic-hard-source-wp/" + hash })
Copy link
Owner

Choose a reason for hiding this comment

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

we should put everything under "phenomic/*". So more "phenomic/hard-source-webpack/" + hash. (wp is confusing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently, there is no namespace in phenomic/*. Perhaps I can move the old webpack static build cache to phenomic/webpack-static-build/* ?

Copy link
Owner

Choose a reason for hiding this comment

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

yep, but we can do that later, no big deal :)

@MoOx
Copy link
Owner

MoOx commented Sep 1, 2016

This is still not working for node?
Maybe we should rename option to "webpack-cache"? And when we will add dll plugin, we might want to have another similar flag so people can toggle one part on and the other off.

Maybe:

  • webpack-dll-cache
  • webpack-hard-cache

Also we should be careful cause coverage is dropping down this days :)

@thangngoc89
Copy link
Contributor Author

Also we should be careful cause coverage is dropping down this days :)

I can add more tests but looks at the diff, it's literally straight forward no logic to test :/

@MoOx
Copy link
Owner

MoOx commented Sep 1, 2016

I guess people might trust more a project that have almost 100%.
We have plenty of "straight forward" files not tested ^^
Anyway, tests will fails (not only on coveralls) if we pass below 70% :)

@thangngoc89
Copy link
Contributor Author

@MoOx PR updated. Good news: By setting cache based on config's hash. We can use hard-source on static build.

➜  docs git:(hard-source-wp) ✗ npm run build

> Phenomic@ build /home/khoa/web/phenomic/docs
> phenomic build

  phenomic:builder ✓ Static build: assets copy completed +0ms
  phenomic:builder ✓ Static build: client build completed +28s
  phenomic:builder ✓ Static build: static build completed +13s
➜  docs git:(hard-source-wp) ✗ npm run build

> Phenomic@ build /home/khoa/web/phenomic/docs
> phenomic build

  phenomic:builder ✓ Static build: assets copy completed +0ms
  phenomic:builder ✓ Static build: client build completed +11s
  phenomic:builder ✓ Static build: static build completed +2s
➜  docs git:(hard-source-wp) ✗ npm run build

> Phenomic@ build /home/khoa/web/phenomic/docs
> phenomic build

  phenomic:builder ✓ Static build: assets copy completed +0ms
  phenomic:builder ✓ Static build: client build completed +11s
  phenomic:builder ✓ Static build: static build completed +1s

@MoOx
Copy link
Owner

MoOx commented Sep 1, 2016

Wowww. That's a serious improvement!

@thangngoc89
Copy link
Contributor Author

I was surprised too. Can't wait to use this .

@MoOx
Copy link
Owner

MoOx commented Sep 1, 2016

I saw you put this default to true. Should we set to false for a while (like if that's experimental?). I can't tell since I didn't used it on any project yet :)

@thangngoc89
Copy link
Contributor Author

thangngoc89 commented Sep 1, 2016

@MoOx mark this as an experimental feature. Set default to false

@MoOx
Copy link
Owner

MoOx commented Sep 1, 2016

Cool! Will merge in a moment!

@MoOx MoOx merged commit b60df76 into master Sep 1, 2016
@MoOx MoOx deleted the hard-source-wp branch September 1, 2016 07:18
MoOx added a commit that referenced this pull request Sep 1, 2016
new HardSourceWebpackPlugin({
cacheDirectory: join(cacheDir, "cacheDirectory"),
environmentPaths: {
root: config.cwd,
Copy link
Owner

Choose a reason for hiding this comment

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

@thangngoc89 why you didn't add "directory: [ "node_modules" ]" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we use node_modules as cache directory. That will make hard-source clean cache everytime

Copy link
Owner

Choose a reason for hiding this comment

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

Hoooo shit. Good point. Maybe we should consider using a .phenomic folder for our cache then (if that can avoid issue)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum. How about os.tmpDir() ?

Copy link
Owner

Choose a reason for hiding this comment

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

yeah maybe. At the end I am not a fan of the cache in node_modules directory. We should investigate for existing solid solutions.

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.

None yet

2 participants