perf(ivy): ngcc - exit early if the targeted package has been compiled#29740
Closed
petebacondarwin wants to merge 2 commits into
Closed
perf(ivy): ngcc - exit early if the targeted package has been compiled#29740petebacondarwin wants to merge 2 commits into
petebacondarwin wants to merge 2 commits into
Conversation
alan-agius4
reviewed
Apr 6, 2019
b69a88e to
a283b5d
Compare
Previously the console logger was being used in integration tests leading to lots of output during test runs.
Previously we always walked the whole folder tree looking for entry-points before we tested whether a target package had been processed already. This could take >10secs! This commit does a quick check of the target package before doing the full walk which brings down the execution time for ngcc in this case dramatically. ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing Compiling @angular/core : fesm2015 as esm2015 Compiling @angular/core : fesm5 as esm5 Compiling @angular/core : esm2015 as esm2015 Compiling @angular/core : esm5 as esm5 Compiling @angular/common/http : fesm2015 as esm2015 Compiling @angular/common/http : fesm5 as esm5 Compiling @angular/common/http : esm2015 as esm2015 Compiling @angular/common/http : esm5 as esm5 Compiling @angular/common/http/testing : fesm2015 as esm2015 Compiling @angular/common/http/testing : fesm5 as esm5 Compiling @angular/common/http/testing : esm2015 as esm2015 Compiling @angular/common/http/testing : esm5 as esm5 real 0m19.766s user 0m28.533s sys 0m2.262s ``` ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing The target entry-point has already been processed real 0m0.666s user 0m0.605s sys 0m0.113s ```
a283b5d to
86c941c
Compare
alan-agius4
reviewed
Apr 6, 2019
| import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point'; | ||
| import {MockLogger} from '../helpers/mock_logger'; | ||
|
|
||
| const _ = AbsoluteFsPath.from; |
Contributor
Author
There was a problem hiding this comment.
In unit tests I have to create AbsoluteFsPaths from strings all over the place, so I got into a habit of aliasing AbsoluteFsPath.from() to _() to make the tests less verbose.
I realise that in this file I only use it once so perhaps that isn't necessary. :-)
Contributor
There was a problem hiding this comment.
I typically associate _ with lodash or underscore 😜.
Contributor
Author
There was a problem hiding this comment.
That's so 1990s :-P
Check out https://github.com/angular/angular/blob/master/packages/compiler-cli/ngcc/test/packages/dependency_host_spec.ts for an example of this being actually useful.
alan-agius4
reviewed
Apr 6, 2019
| return createNewEntryPointFormats ? new NewEntryPointFileWriter() : new InPlaceFileWriter(); | ||
| } | ||
|
|
||
| function hasProcessedTargetEntryPoint( |
IgorMinar
pushed a commit
that referenced
this pull request
Apr 8, 2019
#29740) Previously we always walked the whole folder tree looking for entry-points before we tested whether a target package had been processed already. This could take >10secs! This commit does a quick check of the target package before doing the full walk which brings down the execution time for ngcc in this case dramatically. ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing Compiling @angular/core : fesm2015 as esm2015 Compiling @angular/core : fesm5 as esm5 Compiling @angular/core : esm2015 as esm2015 Compiling @angular/core : esm5 as esm5 Compiling @angular/common/http : fesm2015 as esm2015 Compiling @angular/common/http : fesm5 as esm5 Compiling @angular/common/http : esm2015 as esm2015 Compiling @angular/common/http : esm5 as esm5 Compiling @angular/common/http/testing : fesm2015 as esm2015 Compiling @angular/common/http/testing : fesm5 as esm5 Compiling @angular/common/http/testing : esm2015 as esm2015 Compiling @angular/common/http/testing : esm5 as esm5 real 0m19.766s user 0m28.533s sys 0m2.262s ``` ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing The target entry-point has already been processed real 0m0.666s user 0m0.605s sys 0m0.113s ``` PR Close #29740
gkalpak
reviewed
Apr 9, 2019
Member
gkalpak
left a comment
There was a problem hiding this comment.
Post-merge review, because...why not 😇
|
|
||
| function hasProcessedTargetEntryPoint( | ||
| targetPath: AbsoluteFsPath, propertiesToConsider: string[], compileAllFormats: boolean) { | ||
| const packageJsonPath = AbsoluteFsPath.from(resolve(targetPath, 'package.json')); |
Member
There was a problem hiding this comment.
Since you are concerned with speed. fromUnchecked() should be (negligibly) faster (and sufficient) 😁
Contributor
Author
There was a problem hiding this comment.
I'll fix that up next time I am touching this code.
| } | ||
| } | ||
| // Either all formats need to be compiled and there were none that were unprocessed, | ||
| // Or only the one matching format needs to be compiled but there was at least one matching |
wKoza
pushed a commit
to wKoza/angular
that referenced
this pull request
Apr 17, 2019
Previously the console logger was being used in integration tests leading to lots of output during test runs. PR Close angular#29740
wKoza
pushed a commit
to wKoza/angular
that referenced
this pull request
Apr 17, 2019
angular#29740) Previously we always walked the whole folder tree looking for entry-points before we tested whether a target package had been processed already. This could take >10secs! This commit does a quick check of the target package before doing the full walk which brings down the execution time for ngcc in this case dramatically. ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing Compiling @angular/core : fesm2015 as esm2015 Compiling @angular/core : fesm5 as esm5 Compiling @angular/core : esm2015 as esm2015 Compiling @angular/core : esm5 as esm5 Compiling @angular/common/http : fesm2015 as esm2015 Compiling @angular/common/http : fesm5 as esm5 Compiling @angular/common/http : esm2015 as esm2015 Compiling @angular/common/http : esm5 as esm5 Compiling @angular/common/http/testing : fesm2015 as esm2015 Compiling @angular/common/http/testing : fesm5 as esm5 Compiling @angular/common/http/testing : esm2015 as esm2015 Compiling @angular/common/http/testing : esm5 as esm5 real 0m19.766s user 0m28.533s sys 0m2.262s ``` ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing The target entry-point has already been processed real 0m0.666s user 0m0.605s sys 0m0.113s ``` PR Close angular#29740
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a quick fix for webpack but there are more optimisations to be achieved on the
first time run by replacing the TS module resolver for the initial walk of the tree.
Previously we always walked the whole folder tree looking for
entry-points before we tested whether a target package had been
processed already. This could take >10secs!
This commit does a quick check of the target package before doing
the full walk which brings down the execution time for ngcc in this
case dramatically.