Repository navigation
Ngcc perf improvements - #30525
Closed
petebacondarwin wants to merge 5 commits into
Closed
Ngcc perf improvements#30525petebacondarwin wants to merge 5 commits into
petebacondarwin wants to merge 5 commits into
Conversation
alan-agius4
reviewed
May 18, 2019
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
from
May 18, 2019 09:21
b5829b7 to
f3a5dcd
Compare
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
2 times, most recently
from
May 29, 2019 06:59
f336991 to
402c568
Compare
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
6 times, most recently
from
June 24, 2019 14:35
c29e486 to
dca67d6
Compare
petebacondarwin
marked this pull request as ready for review
June 24, 2019 14:47
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
from
July 3, 2019 19:58
2e711d3 to
c118abb
Compare
JoostK
reviewed
Jul 3, 2019
This message gets called if a format has already been compiled and we only want the first. So the message itself is wrong but it is also not very useful anyway.
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
from
July 4, 2019 10:28
c118abb to
5c23da4
Compare
gkalpak
reviewed
Jul 6, 2019
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
from
July 7, 2019 13:31
4daecad to
d94b8a2
Compare
When profiling ngcc it is notable that a large amount of time is spent dealing with an exception that is thrown (and handled internally by fs) when checking the existence of a file. We check file existence a lot in both finding entry-points and when TS is compiling code. This commit adds a simple cached `FileSystem`, which wraps a real `FileSystem` delegate. This will reduce the number of calls through to `fs.exists()` and `fs.readFile()` on the delegate. Initial benchmarks indicate that the cache is miss to hit ratio for `exists()` is about 2:1, which means that we save about 1/3 of the calls to `fs.existsSync()`. Note that this implements a "non-expiring" cache, so it is not suitable for a long lived `FileSystem`, where files may be modified externally. The cache will be updated if a file is changed or moved via calls to `FileSystem` methods but it will not be aware of changes to the files system from outside the `FileSystem` service. For ngcc we must create a new `FileSystem` service for each run of `mainNgcc` and ensure that all file operations (including TS compilation) use the `FileSystem` service. This ensures that it is very unlikely that a file will change externally during `mainNgcc` processing.
…y-point Previously, ngcc had to walk the entire `node_modules` tree looking for entry-points, even if it only needed to process a single target entry-point and its dependencies. This added up to a few seconds to each execution of ngcc, which is noticeable when being run via the CLI integration. Now, if an entry-point target is provided, only that target and its entry-points are considered rather than the whole folder tree.
Paths can be mapped directly to files, which was not being taken into account when computing `basePaths` for the `EntryPointFinder`s. Now if a `pathMapping` pattern does not exist or is a file, then we try the containing folder instead. Fixes angular#31424
petebacondarwin
force-pushed
the
ngcc-perf-improvements
branch
from
July 8, 2019 09:58
d94b8a2 to
a8d3e0f
Compare
gkalpak
approved these changes
Jul 8, 2019
gkalpak
left a comment
Member
There was a problem hiding this comment.
I think it would be useful to have a test with a path mapped to a parent directory (something like paths: {'@foo': ['../foo']}) to make sure .. segments are resolved correctly.
Otherwise lgtm 👍
Contributor
Author
|
Added a test. |
Contributor
Author
|
Merge assistance - please run presubmit. |
Contributor
jasonaden
pushed a commit
that referenced
this pull request
Jul 9, 2019
When profiling ngcc it is notable that a large amount of time is spent dealing with an exception that is thrown (and handled internally by fs) when checking the existence of a file. We check file existence a lot in both finding entry-points and when TS is compiling code. This commit adds a simple cached `FileSystem`, which wraps a real `FileSystem` delegate. This will reduce the number of calls through to `fs.exists()` and `fs.readFile()` on the delegate. Initial benchmarks indicate that the cache is miss to hit ratio for `exists()` is about 2:1, which means that we save about 1/3 of the calls to `fs.existsSync()`. Note that this implements a "non-expiring" cache, so it is not suitable for a long lived `FileSystem`, where files may be modified externally. The cache will be updated if a file is changed or moved via calls to `FileSystem` methods but it will not be aware of changes to the files system from outside the `FileSystem` service. For ngcc we must create a new `FileSystem` service for each run of `mainNgcc` and ensure that all file operations (including TS compilation) use the `FileSystem` service. This ensures that it is very unlikely that a file will change externally during `mainNgcc` processing. PR Close #30525
jasonaden
pushed a commit
that referenced
this pull request
Jul 9, 2019
…y-point (#30525) Previously, ngcc had to walk the entire `node_modules` tree looking for entry-points, even if it only needed to process a single target entry-point and its dependencies. This added up to a few seconds to each execution of ngcc, which is noticeable when being run via the CLI integration. Now, if an entry-point target is provided, only that target and its entry-points are considered rather than the whole folder tree. PR Close #30525
|
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.
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 PR sits on top of #30591