[clang] Reduce memory usage in AST parent map generation by lazily checking if nodes have been seen (PR #129934)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 13 10:05:56 PDT 2025
higher-performance wrote:
> Interesting... this ends up just being a set-vector with a different identity being stored for the set. So this ends up being ~4 ptr-sizes (IIRC you said that DynTypedNode is 3x the size of a pointer) per-record (though the extra 1 is only for those with memoization data).
Originally 5x (40 bytes vs. 8 bytes), not 3x. But it might be even higher now, since that was using `SmallDenseSet` and this one is using `SmallPtrSet` -- I'm not sure how each one represents a slot in the table, but I imagine the pointer one is better optimized for pointers.
> Did you run the space benchmark here from the original bug?
Yup. With the `SmallVector<1>` I just changed it back to, it's somewhere between 0.6× to 1.3×, depending on the number of elements. It was something like 0.9× to 1.1× with `std::vector`.
> AS FAR AS the `SmallVector`/`SmallDenseSet` vs `std::vector`/`std::set`, the decision here is interesting. `SmallVector` has a pretty massive stack size in exchange for saving on an allocation (it isn't SSO-like, it is more, "size-of-vector + N, so that our allocation is on-stack unless size > N). So it kinda depends on whether we think MOST uses of this are going to be '1' element, vs '0' or '1+' (that is, for the small-vector suggested by @cor3ntin of size 1). If it is almost always going to be 1, I agree with him. If it is almost always going to be 0 or more than 1, std::vector is probably correct (as in the 0 case, std::vector is smaller, and in the 1+ case, the allocation is happening anyway, and we just have dead stack space again).
Yeah I don't know what to expect from user code here, but it seems the difference is small either way -- I'd say let's just go with `SmallVector<1>` now.
https://github.com/llvm/llvm-project/pull/129934
More information about the cfe-commits
mailing list