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.

Sign up for GitHub

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

Jump to bottom

perf(@ngtools/webpack): improve rebuild performance #4188

Merged
merged 1 commit into from
Jan 27, 2017
Merged

perf(@ngtools/webpack): improve rebuild performance #4188

merged 1 commit into from
Jan 27, 2017

Conversation

hansl
Copy link
Contributor

@hansl hansl commented Jan 24, 2017 •
edited
Loading

We need to run full static analysis on the first build to discover routes in node_modules, but in rebuilding the app we just need to look for the changed files.

This introduces a subtle bug though; if the module structure changes, we might be missing lazy routes if those are in modules (exclusively from node_modules) imported but weren't in the original structure.

This is, for now, considered "okay" as it's a relatively rare case. We should probably output a warning though.

Mentions: #1980, #4020, #3315

@hansl
Copy link
Contributor Author

hansl commented Jan 24, 2017

(Hello World is now under 500 msec, so I used Ames instead as benchmark)

$ ng serve
** NG Live Development Server is running on http://localhost:4200. **
Hash: d94faca90a55b4411a5d
Time: 11751ms
chunk    {0} angular.bundle.js (angular) 530 kB {4} [initial] [rendered]
chunk    {1} main.bundle.js (main) 234 kB {3} [initial] [rendered]
chunk    {2} styles.bundle.js (styles) 23.7 kB {4} [initial] [rendered]
chunk    {3} vendor.bundle.js (vendor) 4.63 MB [initial] [rendered]
chunk    {4} inline.bundle.js (inline) 0 bytes [entry] [rendered]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: f3d2f6d5ca2aa459cc43
Time: 808ms
chunk    {0} angular.bundle.js (angular) 530 kB {4} [initial]
chunk    {1} main.bundle.js (main) 234 kB {3} [initial] [rendered]
chunk    {2} styles.bundle.js (styles) 23.7 kB {4} [initial]
chunk    {3} vendor.bundle.js (vendor) 4.63 MB [initial]
chunk    {4} inline.bundle.js (inline) 0 bytes [entry]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: 92dac7d13043681e4bc3
Time: 922ms
chunk    {0} angular.bundle.js (angular) 530 kB {4} [initial]
chunk    {1} main.bundle.js (main) 234 kB {3} [initial] [rendered]
chunk    {2} styles.bundle.js (styles) 23.7 kB {4} [initial]
chunk    {3} vendor.bundle.js (vendor) 4.63 MB [initial]
chunk    {4} inline.bundle.js (inline) 0 bytes [entry]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: 0b0ff5d310fc93b1d838
Time: 674ms
chunk    {0} angular.bundle.js (angular) 530 kB {4} [initial]
chunk    {1} main.bundle.js (main) 234 kB {3} [initial] [rendered]
chunk    {2} styles.bundle.js (styles) 23.7 kB {4} [initial]
chunk    {3} vendor.bundle.js (vendor) 4.63 MB [initial]
chunk    {4} inline.bundle.js (inline) 0 bytes [entry]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: 143afd0e957e3536e044
Time: 672ms
chunk    {0} angular.bundle.js (angular) 530 kB {4} [initial]
chunk    {1} main.bundle.js (main) 234 kB {3} [initial] [rendered]
chunk    {2} styles.bundle.js (styles) 23.7 kB {4} [initial]
chunk    {3} vendor.bundle.js (vendor) 4.63 MB [initial]
chunk    {4} inline.bundle.js (inline) 0 bytes [entry]
webpack: bundle is now VALID.

Rebuild time was, before this PR (from master, including my PR from yesterday):

$ ng serve
** NG Live Development Server is running on http://localhost:4200. **
Hash: c2b1328113c71ae6ef95
Time: 14334ms
chunk    {0} main.bundle.js, main.bundle.map (main) 234 kB {2} [initial] [rendered]
chunk    {1} styles.bundle.js, styles.bundle.map (styles) 23.7 kB {3} [initial] [rendered]
chunk    {2} vendor.bundle.js, vendor.bundle.map (vendor) 4.63 MB [initial] [rendered]
chunk    {3} inline.bundle.js, inline.bundle.map (inline) 0 bytes [entry] [rendered]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: c4ecdf1d4ea8dbeb03d8
Time: 2059ms
chunk    {0} main.bundle.js, main.bundle.map (main) 234 kB {2} [initial] [rendered]
chunk    {1} styles.bundle.js, styles.bundle.map (styles) 23.7 kB {3} [initial]
chunk    {2} vendor.bundle.js, vendor.bundle.map (vendor) 4.63 MB [initial]
chunk    {3} inline.bundle.js, inline.bundle.map (inline) 0 bytes [entry]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: 73d61845b59872e1fc09
Time: 1965ms
chunk    {0} main.bundle.js, main.bundle.map (main) 234 kB {2} [initial] [rendered]
chunk    {1} styles.bundle.js, styles.bundle.map (styles) 23.7 kB {3} [initial]
chunk    {2} vendor.bundle.js, vendor.bundle.map (vendor) 4.63 MB [initial]
chunk    {3} inline.bundle.js, inline.bundle.map (inline) 0 bytes [entry]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: 999b5b13c253f8740157
Time: 1984ms
chunk    {0} main.bundle.js, main.bundle.map (main) 234 kB {2} [initial] [rendered]
chunk    {1} styles.bundle.js, styles.bundle.map (styles) 23.7 kB {3} [initial]
chunk    {2} vendor.bundle.js, vendor.bundle.map (vendor) 4.63 MB [initial]
chunk    {3} inline.bundle.js, inline.bundle.map (inline) 0 bytes [entry]
webpack: bundle is now VALID.
webpack: bundle is now INVALID.
Hash: 8bb8395854fbabb7194c
Time: 2063ms
chunk    {0} main.bundle.js, main.bundle.map (main) 235 kB {2} [initial] [rendered]
chunk    {1} styles.bundle.js, styles.bundle.map (styles) 23.7 kB {3} [initial]
chunk    {2} vendor.bundle.js, vendor.bundle.map (vendor) 4.63 MB [initial]
chunk    {3} inline.bundle.js, inline.bundle.map (inline) 0 bytes [entry]
webpack: bundle is now VALID.

@hansl hansl force-pushed the perf-lazy-routes branch 4 times, most recently from c09100e to e69dd03 Compare January 24, 2017 03:22
Brocco
Brocco suggested changes Jan 24, 2017
View reviewed changes
packages/@ngtools/webpack/src/plugin.ts
@@ -39,7 +40,7 @@ export class AotPlugin implements Tapable {
private _rootFilePath: string[];
private _compilerHost: WebpackCompilerHost;
private _resourceLoader: WebpackResourceLoader;
private _lazyRoutes: { [route: string]: string };
private _lazyRoutes: { [route: string]: string } = Object.create(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Change inline type to use LazyRouteMap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

packages/@ngtools/webpack/src/plugin.ts
}
}
}
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

result will always be an empty object, it's not modified after it's created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops.

@hansl hansl force-pushed the perf-lazy-routes branch 4 times, most recently from 4f43b82 to 877467b Compare January 26, 2017 23:10
@hansl
perf(@ngtools/webpack): improve rebuild performance
80e4ceb 
We need to run full static analysis on the first build to discover routes
in node_modules, but in rebuilding the app we just need to look for the
changed files.

This introduces a subtle bug though; if the module structure changes, we
might be missing lazy routes if those are in modules imported but
weren't in the original structure.

This is, for now, considered "okay" as it's a relatively rare case. We
should probably output a warning though.
@hansl hansl merged commit 7edac2b into angular:master Jan 27, 2017
@hccampos hccampos mentioned this pull request Jan 30, 2017
Closed
MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
@hansl @MRHarrison
perf(@ngtools/webpack): improve rebuild performance (angular#4188)
e82feab 
We need to run full static analysis on the first build to discover routes
in node_modules, but in rebuilding the app we just need to look for the
changed files.

This introduces a subtle bug though; if the module structure changes, we
might be missing lazy routes if those are in modules imported but
weren't in the original structure.

This is, for now, considered "okay" as it's a relatively rare case. We
should probably output a warning though.
@hansl hansl deleted the perf-lazy-routes branch August 2, 2018 19:56
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Brocco Brocco Brocco approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hansl @Brocco @googlebot