dart2js: Destroy some type inference graph edges after type inference.

dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/e85ac191344cdbecfdd39d99ff50209bb3dc837f

Reverted: https://github.com/dart-lang/sdk/commit/9292d1bc2baa05c4ac97ec12d8d922811a181d02

Committed: https://github.com/dart-lang/sdk/commit/ae13804e0913edc17405383a5c829a3ae61fc83f

[sra1, 2016-03-08 02:42:35]

sra@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2016-03-08 16:55:02]

lgtm

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
File pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart (right):

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:1296:
allocatedCalls.clear();
If we are clearing out these collections, it seems strange that we need to do
the cleanupAll on their members as well.

Would it be sufficient to do it only on the 3 maps you have below:
typeInformations, allocatedMaps, allocatedLists (the maps that are used after
inference)?

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:1300:
types.typeInformations.values.forEach((info) => info.clear());
is this still necessary? seems that cleanup would override what we do here.

[sra1, 2016-03-09 19:27:29]

PTAL.

Final heap size is now ~12% smaller on one large program compiled with SSA.

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
File pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart (right):

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:1296:
allocatedCalls.clear();
On 2016/03/08 16:55:01, Siggi Cherem (dart-lang) wrote:
> If we are clearing out these collections, it seems strange that we need to do
> the cleanupAll on their members as well.

The graph has cycles (users and assignments).

> 
> Would it be sufficient to do it only on the 3 maps you have below:
> typeInformations, allocatedMaps, allocatedLists (the maps that are used after
> inference)?

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:1300:
types.typeInformations.values.forEach((info) => info.clear());
On 2016/03/08 16:55:01, Siggi Cherem (dart-lang) wrote:
> is this still necessary? seems that cleanup would override what we do here.

Done.

[Siggi Cherem (dart-lang), 2016-03-09 20:33:23]

lgtm, minor suggestions below

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
File pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart (right):

https://codereview.chromium.org/1776533002/diff/1/pkg/compiler/lib/src/inferr...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:1296:
allocatedCalls.clear();
On 2016/03/09 19:27:29, sra1 wrote:
> On 2016/03/08 16:55:01, Siggi Cherem (dart-lang) wrote:
> > If we are clearing out these collections, it seems strange that we need to
do
> > the cleanupAll on their members as well.
> 
> The graph has cycles (users and assignments).
> 
> > 
> > Would it be sufficient to do it only on the 3 maps you have below:
> > typeInformations, allocatedMaps, allocatedLists (the maps that are used
after
> > inference)?
> 

thanks, makes sense now

https://codereview.chromium.org/1776533002/diff/20001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/type_graph_nodes.dart (right):

https://codereview.chromium.org/1776533002/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_nodes.dart:418: }
or simply:

  _callers ??= <Element, Setlet>{};

https://codereview.chromium.org/1776533002/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_nodes.dart:423: if (_callers == null)
return;
or assert(_callers != null)?

[sra1, 2016-03-09 20:47:21]

Description was changed from

==========
dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.
==========

to

==========
dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e85ac191344cdbecfdd39d99ff50209bb3dc837f
==========

[sra1, 2016-03-09 20:47:22]

Committed patchset #3 (id:40001) manually as
e85ac191344cdbecfdd39d99ff50209bb3dc837f (presubmit successful).

[sra1, 2016-03-09 20:51:09]

https://codereview.chromium.org/1776533002/diff/20001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/type_graph_nodes.dart (right):

https://codereview.chromium.org/1776533002/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_nodes.dart:418: }
On 2016/03/09 20:33:23, Siggi Cherem (dart-lang) wrote:
> or simply:
> 
>   _callers ??= <Element, Setlet>{};

Done.

https://codereview.chromium.org/1776533002/diff/20001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_nodes.dart:423: if (_callers == null)
return;
On 2016/03/09 20:33:23, Siggi Cherem (dart-lang) wrote:
> or assert(_callers != null)?

I'm not sure, since the code is currently a no-op for a missing key.

[sra1, 2016-03-09 22:18:57]

Description was changed from

==========
dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e85ac191344cdbecfdd39d99ff50209bb3dc837f
==========

to

==========
dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e85ac191344cdbecfdd39d99ff50209bb3dc837f

Reverted:
https://github.com/dart-lang/sdk/commit/9292d1bc2baa05c4ac97ec12d8d922811a181d02
==========

[sra1, 2016-03-10 00:45:31]

Description was changed from

==========
dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e85ac191344cdbecfdd39d99ff50209bb3dc837f

Reverted:
https://github.com/dart-lang/sdk/commit/9292d1bc2baa05c4ac97ec12d8d922811a181d02
==========

to

==========
dart2js: Destroy some type inference graph edges after type inference.

Removing the `_assignments` and `users` collections from TypeInformation
nodes causes interior nodes (e.g. Phi, Narrow) to become unreachable and
available for GC. This seems to trim 1%-5% off the final heap of an SSA
compile.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e85ac191344cdbecfdd39d99ff50209bb3dc837f

Reverted:
https://github.com/dart-lang/sdk/commit/9292d1bc2baa05c4ac97ec12d8d922811a181d02

Committed:
https://github.com/dart-lang/sdk/commit/ae13804e0913edc17405383a5c829a3ae61fc83f
==========

[sra1, 2016-03-10 00:45:32]

Committed patchset #5 (id:80001) manually as
ae13804e0913edc17405383a5c829a3ae61fc83f (presubmit successful).