vsavchenko added a comment. In D106136#2889610 <https://reviews.llvm.org/D106136#2889610>, @martong wrote:
> In D106136#2883707 <https://reviews.llvm.org/D106136#2883707>, @vsavchenko > wrote: > >> In D106136#2883100 <https://reviews.llvm.org/D106136#2883100>, @martong >> wrote: >> >>> Thanks for taking your time to take a look. And I accept your statements. >>> Nevertheless, let me explain my motivation. >>> >>> I thought that a class is trivial if it has only one member. And it seemed >>> perfectly logical to not store equivalence classes with one member. I.e `a` >>> equals to `a` does not hold any meaningful information it just takes >>> precious memory. When we add a new constraint we take careful steps to >>> avoid adding a new class with one member. However, when remove dead kicks >>> in, suddenly we end up having classes stored with one member, which is a >>> somewhat inconsistent design IMHO. Perhaps some better documentation could >>> have helped. >> >> Representative symbol gives its equivalence class an ID. We use this ID for >> everything, for comparing and for mapping. Since we live in a persistent >> world, we can't just change this ID at some point, it will basically mean >> that we want to replace a class with another class. So, all maps where this >> class participated (constraints, class, members, disequality) should remove >> the old class and add the new class. This is a huge burden. You need to >> carefully do all this, and bloat existing data structures. >> >> Trivial class is an optimization for the vast majority of symbols that never >> get into any classes. We still need to associate constraints with those. >> This is where implicit Symbol to Class conversion comes in handy. >> >> Now let's imagine that something else is merged into it. We are obliged to >> officially map the symbol to its class, and the class to its members. When >> this happens, it goes into the data structure FOREVER. And when in the >> future, with only one member, instead of keeping something that we already >> have from other versions of the data structure, you decide to remove the >> item from the class map, you actually use more memory when it was there! >> This is how persistent data structures work, different versions of the same >> data structure share data. And I'm not even talking here about the fact >> that you now need to replace the class with one ID with the class with >> another ID in every relationship. >> >> You can probably measure memory footprint in your example and see it for >> yourself. > > Yeah, makes sense. Thanks for taking more time for further explanations. > Still, IMHO, the definition of a **trivial class** needs a clear written > documentation in the code itself, so other developers can easier understand > and maintain this code. I am going to create an NFC patch that summarizes > this discussion in a comment attached to the `isTrivial` function. That's a great idea! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106136/new/ https://reviews.llvm.org/D106136 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
