Skip to content

perf(@ngtools/webpack): only process dependent packages with NGCC#17884

Merged
mgechev merged 1 commit intoangular:masterfrom
alan-agius4:ngcc-use-program-dependencies
Jul 1, 2020
Merged

perf(@ngtools/webpack): only process dependent packages with NGCC#17884
mgechev merged 1 commit intoangular:masterfrom
alan-agius4:ngcc-use-program-dependencies

Conversation

@alan-agius4
Copy link
Copy Markdown
Collaborator

With this change we consume the change in angular/angular#37075 where the NGCC now exposes a new option --use-program-dependencies, to only process packages which are part of the TypeScript program.

From the initial benchmarking the time taken for NGCC to process that dependencies needed for an ng new application went down by 36% from 10526ms to 6708ms.

@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Jun 9, 2020
With this change we consume the change in angular/angular#37075 where the NGCC now exposes a new option `--use-program-dependencies`, to only process packages which are part of the TypeScript program.

From the initial benchmarking the time taken for NGCC to process that dependencies needed for an `ng new` application went down by 36% from 10526ms to 6708ms.
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

36% improvement per line! Looks good to me :-)

@petebacondarwin
Copy link
Copy Markdown
Contributor

The real concern is actually if it slows down the processing of large application because we now scan all their source files as part of the ngcc processing (even if the node_modules have all been processed already).
Does anyone have any large apps that they could test this on?

@alan-agius4
Copy link
Copy Markdown
Collaborator Author

@petebacondarwin let's take this offline.

@alan-agius4
Copy link
Copy Markdown
Collaborator Author

Testing it locally with a dummy 800 component files the NOOP regressed by 200ms.

@alan-agius4 alan-agius4 added the needs: discussion On the agenda for team meeting to determine next steps label Jun 9, 2020
@alan-agius4
Copy link
Copy Markdown
Collaborator Author

alan-agius4 commented Jun 9, 2020

In a large real world application (super-productivity) a NOOP regressed from 1129.807ms to 4685.788ms.

@alan-agius4
Copy link
Copy Markdown
Collaborator Author

On another application which was shared privately with the team. The timings of a noop increased from 687.110ms to 1342.186ms.

@kyliau kyliau added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Jun 15, 2020
@petebacondarwin
Copy link
Copy Markdown
Contributor

I ran some tests on super-productivity using the directory-walker and program-based entry-point finders. See below. I would say that in large projects like this the cost of the program-based finder, is not significant in the bigger picture of the overall build. In fact the variation in the overall build on my machine was so wide that the total build time actually decreased in the "noop" case between the directory-walker and the program-based entry-point finder runs.

directory walker entry-point finder      
  ngcc async create program total
cold 79083 5926 107101
hot 720 5220 64986
hot 959 5449 66637
hot 830 5944 72631
hot (average) 836.3 5537.7 68084.7
program-based entry-point finder      
  ngcc async create program total
cold 86989 8079 113627
hot 3443 5547 68565
hot 3547 5039 66117
hot 3590 4980 64235
hot (average) 3526.7 5188.7 66305.7
  ngcc async create program total
cold diff 7906 2153 6526
  10.00% 36.33% 6.09%
hot diff 2690.3 -349.0 -1779.0
  321.68% -6.30% -2.61%

@dgp1130 dgp1130 added needs: investigation Requires some digging to determine if action is needed and removed needs: discussion On the agenda for team meeting to determine next steps labels Jun 18, 2020
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed needs: investigation Requires some digging to determine if action is needed labels Jul 1, 2020
@mgechev mgechev merged commit 6670af2 into angular:master Jul 1, 2020
@alan-agius4 alan-agius4 deleted the ngcc-use-program-dependencies branch July 1, 2020 18:29
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Aug 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants