ngcc is non-deterministic #35180

jbedard · 2020-02-06T08:07:35Z

🐞 bug report

Affected Package

@angular/*

Is this a regression?

Not sure

Description

I'm finding ngcc is outputting non-deterministic results. Internally I've seen the ivy code swap variable names back and forth between runs, I haven't been able to reproduce this in isolation though. In other cases I'm seeing the exact same code, but with a single line of whitespace at the end which causes a different source-map hash. I've also seen the *.__ivy_ngcc_bak files sometimes being left behind, this normally happens alongside the single line of whitespace.

Non-deterministic results can break caching with tools like bazel. In the case of different variable names it could go a step further and produce different resulting .js files and bust browser caches etc.

Essentially has the same consequences as #34635 but seems less common.

🔬 Minimal Reproduction

Running the following almost always produces different results between some of the ngcc runs.

# Init
yarn add @angular/compiler@9.0.0-rc.14 @angular/compiler-cli@9.0.0-rc.14 @angular/cdk@9.0.0-rc.9 tslib typescript

# Initial ngcc
./node_modules/.bin/ngcc
cp -R node_modules/@angular @angular-0-init

# Redo with fresh node_modules but no changes
rm -Rf node_modules/
yarn
./node_modules-first/.bin/ngcc
cp -R node_modules/@angular @angular-1-rerun

# Redo with new package
yarn add @angular/core@9.0.0-rc.14
./node_modules/.bin/ngcc
cp -R node_modules/@angular @angular-2-new-package

# Redo with new package and out of date yarn.lock
yarn add --no-lockfile @angular/material@9.0.0-rc.9
./node_modules/.bin/ngcc
cp -R node_modules/@angular @angular-3-bad-lock

# Redo with updated yarn.lock
yarn
./node_modules/.bin/ngcc
cp -R node_modules/@angular @angular-4-better-lock

# Redo fresh
rm -Rf node_modules yarn.lock
yarn
./node_modules/.bin/ngcc
cp -R node_modules/@angular @angular-5-redo

🌍 Your Environment

Angular Version:
9.0.0-rc.14, pretty sure it was also happening with 9.0.0-rc.10

The text was updated successfully, but these errors were encountered:

gkalpak · 2020-02-06T18:22:52Z

Argh... 😟

*.__ivy_ngcc_bak are supposed to be left behind, but the empty line thing and variable name swapping sounds suspicious 🤔

Does running ngcc with the --no-async option make the problem go away?
Also, did you notice any patterns regarding the files that are affected? Are all formats affected or particular ones?

jbedard · 2020-02-06T19:49:37Z

I was finding *.__ivy_ngcc_bak was normally NOT being left behind.

Now when trying with --no-async I've also seen __processed_by_ivy_ngcc__ not being added to package.json at all!? Also seeing @angular/compiler/testing/testing.d.ts being updated to have no //# sourceMappingURL and the map file is missing!?

Seems like files across all packages + bundle types are randomly effected. Although in one case it was the testing.{d.ts,js} across all bundle types along with the line of whitespace in bundles/compiler-testing.umd.js

Try running that script above... Currently I'm seeing the biggest changes between step 1+2 (@angular-0-init and @angular-1-rerun folders). Those steps essentially just run the full yarn && ngcc from scratch with no node_modules...

jbedard · 2020-02-11T06:57:45Z

@gkalpak have you reproduced anything?

After upgrading to v9.0.0 I'm consistently seeing the inconsistent variable names, but now none of the *.__ivy_ngcc_bak or __processed_by_ivy_ngcc__ diffs. Using the following and diffing @angular/* between each run:

# Init
yarn add rxjs zone.js @angular/core @angular/platform-browser @angular/common @angular/forms @angular/animations @angular/compiler @angular/compiler-cli @angular/cdk @angular/material tslib typescript@3.7

# Initial ngcc
node_modules/.bin/ngcc --properties es2015 browser module main
cp -R node_modules/@angular @angular-0-init

# Redo as-is
node_modules/.bin/ngcc --properties es2015 browser module main
cp -R node_modules/@angular @angular-1-rerun

# Redo with fresh node_modules but no changes
rm -Rf node_modules/
yarn
node_modules/.bin/ngcc --properties es2015 browser module main
cp -R node_modules/@angular @angular-2-rerun-fresh

# Redo fresh
rm -Rf node_modules yarn.lock
yarn
node_modules/.bin/ngcc --properties es2015 browser module main
cp -R node_modules/@angular @angular-3-rerun-drop-lock

gkalpak · 2020-02-19T16:57:48Z

@jbedard, I tried with the --no-async option a couple of times and the resulting files were identical. I believe it is expected that the auto-generated var names will be different when running in parallel mode, because the order in which each dependency is processed is not deterministic.

With --no-async on the other hand, the dependencies are always processed in the same order and thus the operation should be idempotent.

jbedard · 2020-02-20T06:23:25Z

Idk if I'd call that "expected"... I think running npm/yarn with the same lock file, cached or not, should produce the same result every single time.

Is there any real reason other then sharing a single counter somewhere? Why not one per-file instead of global? Seems like that's what the import * as ɵngcc# is doing already..?

gkalpak · 2020-02-20T12:51:57Z

I think running npm/yarn with the same lock file, cached or not, should produce the same result every single time.

If you added a postinstall script that changes node_modules/ in a non-deterministic way, that's your fault 😛 (Use the --no-async option 😁)

Why not one per-file instead of global?

I imagine it wouldn't be super straight-forward (I might be wrong though), but it should be doable.
Leaving it here as a potential future improvement. If anyone feels like picking it up, please do 😉

petebacondarwin · 2020-03-25T12:53:05Z

Tracking at FW-2022

petebacondarwin · 2020-03-31T16:41:04Z

I just tried the reproduction given in #35180 (comment) and I cannot see any difference between the generated files. But I wonder if it is timing dependent and so I was just lucky?

The versions of Angular etc I am testing with are:

Angular CLI: 9.1.0
Node: 12.16.1
OS: darwin x64

Angular: 9.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.0
@angular-devkit/build-angular     0.901.0
@angular-devkit/build-optimizer   0.901.0
@angular-devkit/build-webpack     0.901.0
@angular-devkit/core              9.1.0
@angular-devkit/schematics        9.1.0
@ngtools/webpack                  9.1.0
@schematics/angular               9.1.0
@schematics/update                0.901.0
rxjs                              6.5.4
typescript                        3.8.3
webpack                           4.42.0

@jbedard - could you take another look?

jbedard · 2020-03-31T18:20:02Z

I reproduced it running that script. @angular-0-init and @angular-2-rerun-fresh produced different results for quite a few files, only in the cdk and material though.

My package package.json is:

{
  "dependencies": {
    "@angular/animations": "^9.1.0",
    "@angular/cdk": "^9.2.0",
    "@angular/common": "^9.1.0",
    "@angular/compiler": "^9.1.0",
    "@angular/compiler-cli": "^9.1.0",
    "@angular/core": "^9.1.0",
    "@angular/forms": "^9.1.0",
    "@angular/material": "^9.2.0",
    "@angular/platform-browser": "^9.1.0",
    "rxjs": "^6.5.4",
    "tslib": "^1.11.1",
    "typescript": "3.7",
    "zone.js": "^0.10.3"
  }
}

petebacondarwin · 2020-03-31T21:07:31Z

I found the problem. These non-deterministic variables are generated by the BindingScope class, which is a node in a tree of scopes starting with the ROOT_SCOPE at the top. The ROOT_SCOPE has the top level counter that is used to generate new names, but is a singleton object for the whole life-time of the app.

Normally this is fine but for ngcc in async mode, we are creating multiple worker processes, each of which gets its own ROOT_SCOPE. And so depending upon the tasks (and the order of them) that are assigned to each worker we might get different variable names generated.

I think the solution is to recreate the ROOT_SCOPE for each file being processed in the same way that ConstantPool is.

Previously we had a singleton `ROOT_SCOPE` object, from which all `BindingScope`s derived. But this caused ngcc to produce non-deterministic output when running multiple workers in parallel, since each process had its own `ROOT_SCOPE`. In reality there is no need for `BindingScope` reference names to be unique across an entire application (or in the case of ngcc across all the libraries). Instead we just need uniqueness within a template. This commit changes the compiler to create a new root `BindingScope` each time it compiles a component's template. Resolves angular#35180

#36362) Previously we had a singleton `ROOT_SCOPE` object, from which all `BindingScope`s derived. But this caused ngcc to produce non-deterministic output when running multiple workers in parallel, since each process had its own `ROOT_SCOPE`. In reality there is no need for `BindingScope` reference names to be unique across an entire application (or in the case of ngcc across all the libraries). Instead we just need uniqueness within a template. This commit changes the compiler to create a new root `BindingScope` each time it compiles a component's template. Resolves #35180 PR Close #36362

angular-automatic-lock-bot · 2020-05-10T16:08:35Z

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.}

gkalpak added the comp: ngcc label Feb 6, 2020

ngbot bot added this to the needsTriage milestone Feb 6, 2020

petebacondarwin added freq2: medium risk: medium type: bug/fix labels Feb 8, 2020

jbedard mentioned this issue Feb 11, 2020

performance: poor cache performance bazelbuild/rules_nodejs#1501

Closed

gkalpak self-assigned this Feb 19, 2020

petebacondarwin assigned petebacondarwin and unassigned gkalpak Mar 31, 2020

petebacondarwin mentioned this issue Apr 1, 2020

refactor(compiler): create a new root BindingScope for each template #36362

Closed

atscott closed this as completed in e526f74 Apr 9, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ngcc is non-deterministic #35180

ngcc is non-deterministic #35180

jbedard commented Feb 6, 2020 •

edited

gkalpak commented Feb 6, 2020

jbedard commented Feb 6, 2020

jbedard commented Feb 11, 2020

gkalpak commented Feb 19, 2020

jbedard commented Feb 20, 2020

gkalpak commented Feb 20, 2020

petebacondarwin commented Mar 25, 2020

petebacondarwin commented Mar 31, 2020

jbedard commented Mar 31, 2020

petebacondarwin commented Mar 31, 2020 •

edited

angular-automatic-lock-bot bot commented May 10, 2020

ngcc is non-deterministic #35180

ngcc is non-deterministic #35180

Comments

jbedard commented Feb 6, 2020 • edited

🐞 bug report

Affected Package

Is this a regression?

Description

🔬 Minimal Reproduction

🌍 Your Environment

gkalpak commented Feb 6, 2020

jbedard commented Feb 6, 2020

jbedard commented Feb 11, 2020

gkalpak commented Feb 19, 2020

jbedard commented Feb 20, 2020

gkalpak commented Feb 20, 2020

petebacondarwin commented Mar 25, 2020

petebacondarwin commented Mar 31, 2020

jbedard commented Mar 31, 2020

petebacondarwin commented Mar 31, 2020 • edited

angular-automatic-lock-bot bot commented May 10, 2020

jbedard commented Feb 6, 2020 •

edited

petebacondarwin commented Mar 31, 2020 •

edited