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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
