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

AOT compilation using ngtools/webpack #5952

Merged
merged 9 commits into from Jul 3, 2017
Merged

AOT compilation using ngtools/webpack #5952

merged 9 commits into from Jul 3, 2017

Conversation

deepu105
Copy link
Member

@deepu105 deepu105 commented Jun 19, 2017

  • Please make sure the below checklist is followed for Pull Requests.

  • Travis tests are green

  • Tests are added where necessary

  • Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

cc @sendilkumarn

This is causing some sort of infinite loop in ngc

@deepu105 deepu105 mentioned this pull request Jun 19, 2017
21 tasks
@sendilkumarn
Copy link
Member

infinite loops ?

@sendilkumarn
Copy link
Member

sendilkumarn commented Jun 19, 2017

check this
and it happens after mergejson is initialized... I dont see any error in my system either.

@deepu105
Copy link
Member Author

But this doesnt happen in master only in this branch

new webpack.optimize.UglifyJsPlugin({
beautify: false,
comments: false,
sourcMap: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

@sendilkumarn is this a typo? shouldnt this be sourceMap

@deepu105
Copy link
Member Author

@sendilkumarn I found the problem now need to find a fix

@deepu105
Copy link
Member Author

@jhipster/developers I'm giving up on this I spend hours on this but couldn't get it working. here is what I did so far
So the idea of using ngtools to do AOT build is so that the generated AOT files go through the same webpack pipeline and hence ngc shouldn't be used with ngtools.

But the problem is that somehow its unable to find the ngfactory* files in memory. I tried different samples online https://github.com/blacksonic/angular2-aot-cli-webpack-plugin, https://github.com/seanlandsman/webpack-ngtools-blog etc, and tried to replicate the settings but no luck. I'm unable to figure out what is going wrong. I think we need an angular experts help here @jdubois do you know anyone who could help here?

@mraible
Copy link
Contributor

mraible commented Jun 25, 2017

Maybe @TheLarkInn can help?

@jdubois
Copy link
Member

jdubois commented Jun 25, 2017

@deepu105 not sure - the best solution would be to ask for help on Twitter, could you do a message pointing to your explanation here? I will RT it, of course.

@deepu105
Copy link
Member Author

@jdubois btw I think this is the only thing blocking Angular out of beta

@deepu105
Copy link
Member Author

I did a tweet

@manekinekko
Copy link

@deepu105 what are the steps to reproduce the issue you're having?

@deepu105
Copy link
Member Author

@manekinekko I'll push a repo with sample code now

@manekinekko
Copy link

great. thanks.

@deepu105
Copy link
Member Author

@manekinekko here is the repo https://github.com/deepu105/ngtools-sample
Run the below to reproduce the issue

yarn install
yarn webpack:prod

The webpack config is under the ./webpack folder. Angular code is under ./src/main/webapp/app folder. tsconfig files are at ./

@deepu105
Copy link
Member Author

Hi @manekinekko did you manage to take a look at this?

@manekinekko
Copy link

manekinekko commented Jun 30, 2017

Yep, I managed to find some time yesterday evening ^^

The issue was related to the typescript configuration in tsconfig-aot.json .
Here is the config that gets this working:

{
    "compilerOptions": {
        "target": "es5",
        "module": "es2015",
        "moduleResolution": "node",
        "sourceMap": true,
        "emitDecoratorMetadata": true,
        "experimentalDecorators": true,
        "removeComments": false,
        "noImplicitAny": false,
        "suppressImplicitAnyIndexErrors": true,
        "outDir": "target/www/app",
        "lib": ["es2015", "dom"],
        "typeRoots": [
            "node_modules/@types"
        ]
    },

    "files": [
        "./src/main/webapp/app/app.module.ts",
        "./src/main/webapp/app/app.main-aot.ts"
    ],

    "angularCompilerOptions": {
        "genDir": "target/aot",
        "entryModule": "src/main/webapp/app/app.module#Jh4TestAppModule",
        "skipMetadataEmit": true
    }
}

Please note that I'm using the entryModule property in the tsconfig-aot.json. This is just for convenience. You can leave it in the webpack config.

Also, it seems that webpack has to optimize a lot of files. As a consequence, it requires a lot of memory, so I had to bump up the memory size of node before running webpack:

node --max_old_space_size=4000 node_modules/webpack/bin/webpack.js -p --config webpack/webpack.prod.js --progress --profile --bail

And the build process was abnormally long (14min)

image

Here is the devtools report regarding the application bootstrap in AOT vs JIT

AOT

image

JIT

image

cc @deepu105 @jdubois

@deepu105
Copy link
Member Author

@manekinekko Thanks a ton for taking the time to look into this. We were also able to get compilation working with something similar but faced the same issue of memory timeout and abnormally long build. Any idea on what would be making the build take such long? Initially, I thought it was some kind of cyclic dependency but couldn't find any. On the other hand compiling just with ngc takes around 60 seconds. Would you advice sticking to using ngc directly instead of using ngtools?

@sendilkumarn
Copy link
Member

wow @manekinekko Thanks a lot for taking the time to look into this.

but 14 mins :( I think we can manually compile and use it rather than using ngtools. one less pain :p

@deepu105
Copy link
Member Author

@sendilkumarn if you have time can you clone the branch and try to do it with just ngc without ngtools?

@manekinekko
Copy link

I managed to bring the build down to 2m34s by disabling the source map in prod mode (devtool: 'eval') and setting sourceMap: false, of webpack.optimize.UglifyJsPlugin in webpack.prod.js. Of course this is not a solution.

One intriguing thing is that processing the html and scss is what takes too long:
image

@deepu105 The official tool for building Angular in AOT mode is the Angular Compiler which CLI is ngc. The webpack plugin uses a private API of the Angular Compiler. So in theory, yes as @sendilkumarn suggested, you can just use ngc to build Angular in AOT. But then I guess you'll need to sync the two processes: ngc + webpack (since you still need to build/optimise the other assets).

Question: I'm interested in hearing why you didn't use the Angular CLI with this generator?

@sendilkumarn
Copy link
Member

sendilkumarn commented Jun 30, 2017

Question: I'm interested in hearing why you didn't use the Angular CLI with this generator?

@manekinekko infact we tried #5661

@manekinekko
Copy link

@sendilkumarn I see. sounds fair. I'd argue however that integrating the Angular CLI would make this generator compatible with the official stack.

@deepu105
Copy link
Member Author

deepu105 commented Jul 2, 2017

@jhipster/developers So finally I got this working with reasonable build times and good performance

Default app builds in Dev : Done in 30.11s. (12+ MB)
Default app builds in Prod with AOT : Done in 95.82s. (402KB gzip)
image

An app with lot of entities (23 of our travis sample test entities) :
App builds in Dev : Done in 50.02. (14+ MB)
image

App builds in Prod with AOT : Done in 808.20s. (656KB gzip)
image

Thats around 13 mins for huge app. But as per official docs https://angular.io/guide/aot-compiler#jit-in-development-aot-in-production its seems to be expected

Today AOT compilation and tree shaking take more time than is practical for development. That will change soon. For now, it's best to JIT compile in development and switch to AOT compilation before deploying to production.
So I guess we may not be able to spped it up further even with Angular CLI. See angular/angular-cli#6516

I used the ngc-webpack plugin for this and thanks to sample from https://github.com/AngularClass/angular-starter

@manekinekko thanks a lot for your inputs and help. WDYT of the implementation I have done?

@deepu105
Copy link
Member Author

deepu105 commented Jul 2, 2017

@sendilkumarn @jdubois please test it

@deepu105
Copy link
Member Author

deepu105 commented Jul 2, 2017

I'm gonna stop 1 of the 2 builds trigerred

@sendilkumarn sendilkumarn requested review from sendilkumarn and removed request for sendilkumarn July 3, 2017 05:24
@sendilkumarn
Copy link
Member

Let me have a look.

@deepu105 but 4xx is slightly less than what we have now right ?

@sendilkumarn
Copy link
Member

@deepu105 Good Job 👏

Wow, finally it works but still with ~115 seconds. But as we discussed this earlier, I am fine with that.

@sendilkumarn
Copy link
Member

there are time outs in build can you check that ?

screen shot 2017-07-03 at 2 01 55 pm

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017

@sendilkumarn I was expecting that. Those 2 builds have more than 20 entities and they run the prod build. SO build will take more than 15 mins. @pascalgrimaud is it possible to increase the timeout in travis to 20 mins for these builds?

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017

Btw @jdubois what do you think of the build times? is it acceptable? Anyways with AOT I dont think we have any way to speed it up further(Unless there are improvements in Angular/webpack) so the more entities or front end code you have the slower the prod build would be. One more way to reduce the build time by few mins would be to disable sourcemaps in production

@pascalgrimaud
Copy link
Member

Not sure we can change the timeout on Travis, but I will have a look
Good jobs guys !

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017

@pascalgrimaud
Copy link
Member

Oh nice, I didn't know this, thanks @deepu105

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017

@pascalgrimaud btw I also found that Travis has an inbuilt command to retry 3 times on failure (we do that manually for protractor right?)

@pascalgrimaud
Copy link
Member

exact @deepu105
But I wanted to avoid a retry when there is a real error, only retry for random failures with protractor

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017 via email

@jdubois
Copy link
Member

jdubois commented Jul 3, 2017

@deepu105 I'm not sure that sourcemaps in prod are a good idea anyway -> yes can you remove them? Or make them optional, and disabled by default?

I'm really sad to discover one more time that Angular does not work well for real apps in production, which was the whole point of switching from AngularJS.

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017

@jdubois source maps are disabled by default. It's commented so people can enable it if required. Maybe we can add a line of documentation regarding it in using webpack/angular page

@jdubois
Copy link
Member

jdubois commented Jul 3, 2017

@deepu105 if I do a prod build of my own sample app (only 3 entities):

  • in app I still get app/src and app/target folders, and they take about 5Mb, could you remove them?
  • my total build (with Java and all tests) took 2:17 minutes, so that's totally fine for me

@jdubois
Copy link
Member

jdubois commented Jul 3, 2017

@deepu105 -> your build passed, and as far as I'm concerned this all looks OK. I tested against an "old" AngularJS 1 app, and performance isn't great (in fact it's worse!!), but it's basically working OK and I had no issue.

-> let's merge this!!!!! WDYT? I'll let you push the button :-)

@deepu105
Copy link
Member Author

deepu105 commented Jul 3, 2017

@jdubois yes lets merge this. About the folders, are you talking about the build|target folder? if so they are created during AOT compile. I need to see if there is a way to remove it within webpack else after build we need to remove it using rimraf

@deepu105 deepu105 merged commit 384630a into master Jul 3, 2017
@deepu105 deepu105 deleted the ngtools branch July 3, 2017 16:38
@jdubois jdubois modified the milestone: 4.6.0 Jul 6, 2017
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

Successfully merging this pull request may close these issues.

None yet

6 participants