Many duplicate InterfaceType objects in dart2js modular compile final phase

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.

https://dart.dev

BSD 3-Clause "New" or "Revised" License

10.21k stars 1.57k forks source link

Many duplicate InterfaceType objects in dart2js modular compile final phase #44482

Open rakudrama opened 3 years ago

rakudrama commented 3 years ago

In the final phase of a modular dart2js compile of a large ACX app there are 5.6M Kernel ir.InterfaceType objects. Total size is 172.7MB, (retained 264MB).

Nearly half of the instances are for eight classes, only one of which is generic. class	count
String	798047
bool	520317
int	511059
Object	487435
List<>	205816
Type	148029
HtmlElement	145660
RenderView	112331

It seems like the 798k instances for String should be replaced by at most four instances for the different Nullability values (nullability is stored on the instance).

A similar pattern occurs with the dart2js DartType version - many occurrences of some basic types.

I suspect that DartTypes are serialized structurally.

rakudrama commented 3 years ago

If I de-duplicate the 5.6M kernel AST InterfaceType instances, there are about 100K unique instances.This is about 50x more instances that necessary. (instances.toSet().length returns 102307)

@jensjoha could the Kernal AST reader be configured to intern types?

jensjoha commented 3 years ago

First thoughts:

Could/would we ever change them (as that changing one would destroy all the other ones)? (e.g. InterfaceType has a non-final field Reference className --- I'm not sure why it's non-final (update: making it final only seems to give errors in mixin_deduplication.dart in pkg/vm)).
What is the consequence of doing it? Does it take more time or less time? How much are we talking? What about storage? And memory?
Could the VM benefit as well? (e.g. would it make sense to do it when we write the dill?)

To answer the direct question: No, it's not currently possible to do that. It could be easily added though --- I guess we could start by doing it as an experiment?

/cc @johnniwinther --- thoughts on this?

rakudrama commented 3 years ago

Storage for an interning table is worthwhile:

The InterfaceType objects is 32 bytes (4 words). Some instances share a const <DartType>[] for the typeArguments, but other instances 'own' an empty fixed-length _List, for another 32 bytes. A LinkedHashMap has worst-case size of 6 words (x8 bytes) per entry + a small constant. Best case storage today, assuming fully shared typeArguments, is 325.6M = ~180MB Worst case storage with interning map is (103K (32+48+32)) = ~11.5MB, or 6% of above.

johnniwinther commented 3 years ago

I think we should try to make InterfaceType immutable. This is how generally use it. I'll look into the mixin deduplication issue.