[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dexonsmith added a comment. @nikic, great news! Thanks for doing the detailed analysis. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621 ___ cfe-commits mailing

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

nikic added a comment. In D77621#2000237 , @nikic wrote: > > Perhaps you could move the value computation into a constexpr variable & > > just return that as needed. (could be a static local constexpr, I guess - > > to avoid the issues around linkage of

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dexonsmith added a comment. In D77621#2000237 , @nikic wrote: > Okay, I'm basically fine with that, if it is our stance that SmallVector > should always be preferred over std::vector. Not really related to this > revision, but it would probably help to d

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

nikic added a comment. In D77621#157 , @dblaikie wrote: > @nikic any sense of the noise floor/level on these measurements? It doesn't > /look/ like there's much left in this that would cause problems. & I assume > these measurements were made on an o

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dblaikie added a comment. In D77621#1999757 , @browneee wrote: > I resubmitted the report_fatal_error checks again under D77601 > > > http://llvm-compile-time-tracker.com/compare.php?from=7375212172951d2fc283c81d03c1a8588

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee added a comment. I resubmitted the report_fatal_error checks again under D77601 http://llvm-compile-time-tracker.com/compare.php?from=7375212172951d2fc283c81d03c1a8588c3280c6&to=a30e7ea88e75568feed020aedae73c52de35&stat=max-rss http://llvm-compile-t

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dblaikie added a comment. > because it causes a 1% compile-time and memory usage regression. Yeah, some memory regression is expected and, in my opinion, acceptable for the change. The compile time regression presumably came from the changes to the report_fatal_error handling in SmallVector

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dexonsmith added a comment. In D77621#1995673 , @browneee wrote: > Thanks for the revert explanation and notes, nikic. > > @dexonsmith what is your current thinking on going back to the original > std::vector approach? SmallVector has only been limited

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee updated this revision to Diff 256621. browneee added a comment. Change to suggested approach: size and capacity type conditionally larger for small element types. Also incorporate https://reviews.llvm.org/D77601 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dblaikie added a comment. In D77621#1970015 , @dexonsmith wrote: > In D77621#1968647 , @browneee wrote: > > > Do we want to increase the complexity of SmallVector somewhat? or do we > > want to keep the limit and a

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dexonsmith added a comment. In D77621#1968647 , @browneee wrote: > Do we want to increase the complexity of SmallVector somewhat? or do we want > to keep the limit and affirm SmallVector is for small things? I don't think we should limit `SmallVector` t

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee added a comment. In D77621#1968437 , @dexonsmith wrote: > This is thanks to a commit of mine that shaved a word off of `SmallVector`. > Some options to consider: > > 1. Revert to word-size integers (`size_t`? `uintptr_t`?) for Size and > Capaci

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee added inline comments. Comment at: llvm/include/llvm/Bitstream/BitstreamWriter.h:19 #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" RKSimon wrote: > Can this be dropped? It is still used to construct re

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee updated this revision to Diff 255891. browneee added a comment. Fix formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621 Files: clang-tools-extra/clang-doc/Serialize.cpp clang-tools-extr

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee updated this revision to Diff 255875. browneee marked 2 inline comments as done and an inline comment as not done. browneee added a comment. Build fixes in additional projects. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Requesting changes just to be sure we consider the other options. I don't think it's good that `SmallVector` is no longer useful for large byte streams; I would prefer to fi

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

dexonsmith added a comment. This is thanks to a commit of mine that shaved a word off of `SmallVector`. Some options to consider: 1. Revert to word-size integers (`size_t`? `uintptr_t`?) for Size and Capacity for small-enough types. Could be just if `sizeof(T)==1`. Or maybe just for `char`

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee updated this revision to Diff 255856. browneee added a comment. Fix build errors. Missed -DLLVM_ENABLE_PROJECTS in previous local test builds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621 Files:

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee updated this revision to Diff 255831. browneee added a comment. Fix more build errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621 Files: clang/include/clang/Serialization/ASTWriter.h clang

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

browneee updated this revision to Diff 255773. browneee marked an inline comment as done. browneee added a comment. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. Fix build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o