[llvm-commits] [PATCH] Make Inits FoldingSetNodes

[Jakob Stoklund Olesen]

On Jul 13, 2011, at 3:33 PM, David Greene wrote:

> Here is the patch to turn Inits into FoldingSetNodes.  It depends on
> some mechanical patches to remove non-const iterators and to make Inits
> const, so it won't apply to trunk just yet.
> 
> Is someone able to review it?  Thanks!

The use of factory methods is awesome. That is definitely the way to go.

I agree with Chris that FoldingSet is overkill for this, though.

> +  Init(const FoldingSetNodeID &ID) : FastFoldingSetNode(ID) {}

This adds 64 bytes of overhead to an IntInit that was 16 bytes before. That is going in the wrong direction.

UnsetInit and BitInit should be singletons (or doubletons if that is a word). DenseMap<int64_t,IntInit*> is hard to beat for interning IntInits.

For StringInit and CodeInit, I am not sure interning is even necessary. You definitely want to have an empty string singleton, but other than that, how many duplicate strings are there really?

> +  static void ReleaseMemory() {
> +    InitAllocator.Reset();
> +  }


I don't think there is any reason to release memory. This function leaks a ton anyway since it doesn't call destructors, and it is only called immediately before exiting the process. It is very hard to beat _exit(2) for speed of deallocating complex data structures.

> +  static BumpPtrAllocator InitAllocator;

And if you are not releasing memory, the BumpPtrAllocator doesn't buy you much over plain new. For small objects like the Inits, malloc() is pretty fast. It is free() that is slow.


ListInits are interesting. We allocate a ton of them in ListRecTy::convertValue() which is a little too naive. It should recognize the cases where it didn't change anything, and just return the input.

I think you could eliminate almost all duplicate ListInits by:

- Teaching ListRecTy::convertValue() to avoid creating gratuitous duplicates.

- Interning empty lists by element type.

- Interning singleton lists by their single element.

That would likely have less overhead than using a FoldingSet. I don't think the multi-element lists have enough duplicates that it is worth interning them.


I was looking at TableGen's memory usage a while ago. I attacked the RecTy instances instead of the Inits, but I used the same basic idea of interning. TableGen is allocating some 200k ListRecTy instances when parsing ARM.td ;-)

I have a patch that interns all the RecTy objects. It reduces TableGen heap usage from 24 MB to 16 MB IIRC. That is a lot in relative terms, but not in absolute terms, and it didn't improve parse time by much. I didn't think it was worth the churn at the time, but since you are churning in the same code now, I can send you the patch if you are interested.

/jakob