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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

inconsistent __processed_by_ivy_ngcc__ order breaks caching #34635

Closed
jbedard opened this issue Jan 3, 2020 · 5 comments
Closed

inconsistent __processed_by_ivy_ngcc__ order breaks caching #34635

jbedard opened this issue Jan 3, 2020 · 5 comments
Assignees
@gkalpak
Labels
area: bazel Issues related to the published `@angular/bazel` build rules freq1: low state: has PR type: bug/fix
Milestone
v9.x-candidates

Comments

@jbedard
Copy link
Contributor

jbedard commented Jan 3, 2020

馃悶 bug report

Affected Package

@angular/*, @angular/bazel users especially

Is this a regression?

Yes, in ivy at least.

Description

#32427 made the ngcc tasks run in parallel (FYI @gkalpak 馃憢). I assume this is what causes the __processed_by_ivy_ngcc__ entries in package.json to be defined in a random order, causing inconsistent package.json content. Tools such as bazel which use file hashes for caching purposes will then get cache misses (almost) every time ngcc runs on a package.

馃敩 Minimal Reproduction

Run yarn install a couple times and observe the package.json files being inconsistent.

If using bazel you can use --execution_log_json_file=executions.log on a clean build a few times and see the difference, the hashes of many @angular/* package.json files changes.

Angular Version:

9.0.0-rc.7, I think it also existed back to rc.0 and earlier.
@jbedard jbedard mentioned this issue Jan 3, 2020
Closed
@AndrewKushnir AndrewKushnir added area: bazel Issues related to the published `@angular/bazel` build rules comp: ngcc labels Jan 4, 2020
@ngbot ngbot bot modified the milestone: needsTriage Jan 4, 2020
@alxhub alxhub modified the milestones: needsTriage, v9-candidates Jan 7, 2020
@petebacondarwin
Copy link
Member

@jbedard - can you give some more details of what you mean by "causing inconsistent package.json content."? What is different in the content per run? Each package.json should only be touched once, whether ngcc is running in parallel mode or not.

Are you saying that the order of the entry-points within the __processed_by_ivy_ngcc__ is different? If so, then a simple fix might be to sort these entry-points alphabetically to ensure they are stable?

@gkalpak
Copy link
Member

gkalpak commented Jan 9, 2020 •
edited

Yes, that's what's happening. The fix is indeed to sort the properties in __processed_by_ivy_ngcc__ alphabetically. (I was going to work on this today, but forgot to assign myself 馃榿)

@jbedard
Copy link
Contributor Author

jbedard commented Jan 9, 2020

Yeah that's correct, and simply sorting them before writing to package.json would fix it.

@petebacondarwin each package.json is touched once per fresh node_modules. Locally that's not often, but across machines it is. So a bazel cache shared between devs becomes useless (or at least not a "shared" cache anymore). Or in our case our CI didn't retain any state between runs so there was a fresh node_modules between runs, and those npm packages are near the root of the dep tree so them changing breaks almost all caching...

gkalpak added a commit to gkalpak/angular that referenced this issue Jan 20, 2020
@gkalpak
fix(ngcc): update package.json deterministically
9b1d9df 
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order of processing each format.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes angular#34635
@gkalpak gkalpak mentioned this issue Jan 20, 2020
Closed
@gkalpak
Copy link
Member

gkalpak commented Jan 20, 2020 •
edited

I was a little reluctant to order everything alphabetically (and thus modify the package.jsons so significantly), so I implemented a more conservative approach in #34870, ensuring that the relative properties added to mark an entry-point as processed is deterministic (i.e. independent of the order in which each format's processing completes).

If you want to try it out, you can get the build artifacts from here, @jbedard.
(Instructions on how to use them can be found here.)

gkalpak added a commit to gkalpak/angular that referenced this issue Jan 20, 2020
@gkalpak
fix(ngcc): update package.json deterministically
9ccfaec 
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes angular#34635
gkalpak added a commit to gkalpak/angular that referenced this issue Jan 21, 2020
@gkalpak
fix(ngcc): update package.json deterministically
3e7e86d 
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes angular#34635
gkalpak added a commit to gkalpak/angular that referenced this issue Jan 21, 2020
@gkalpak
fix(ngcc): update package.json deterministically
876b3a3 
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes angular#34635
AndrewKushnir pushed a commit that referenced this issue Jan 23, 2020
@gkalpak @AndrewKushnir
fix(ngcc): update package.json deterministically (#34870)
a10d2a8 
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes #34635

PR Close #34870
@jbedard jbedard mentioned this issue Feb 6, 2020
Closed
sonukapoor pushed a commit to sonukapoor/angular that referenced this issue Feb 13, 2020
@gkalpak @sonukapoor
fix(ngcc): update package.json deterministically (angular#34870)
b2c03c5 
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes angular#34635

PR Close angular#34870
@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 Feb 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@gkalpak gkalpak

Labels
area: bazel Issues related to the published `@angular/bazel` build rules freq1: low state: has PR type: bug/fix
Projects
None yet
Milestone
v9.x-candidates
Development

Successfully merging a pull request may close this issue.

fix(ngcc): update package.json deterministically gkalpak/angular
6 participants
@petebacondarwin @jbedard @JoostK @alxhub @gkalpak @AndrewKushnir