[PATCH] D137918: [nfc] move expensive parts of Hashing.h into cpp file

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 2 13:46:09 PST 2022


dblaikie added inline comments.


================
Comment at: llvm/include/llvm/ADT/Hashing.h:51
 #include "llvm/Support/type_traits.h"
 #include <algorithm>
 #include <cassert>
----------------
Trass3r wrote:
> dblaikie wrote:
> > Trass3r wrote:
> > > Trass3r wrote:
> > > > It'd be good to get rid of this completely since it shows up at the top of the expensive includes list but there's a non-trivial usage left below.
> > > https://github.com/Trass3r/llvm-project/actions/runs/3458979320/jobs/5773908563#check-step-6
> > > ```
> > > *** Expensive headers:
> > > 1461737 ms: lvm/include/llvm/ADT/Hashing.h (included 5485 times, avg 266 ms), included via:
> > > 1269781 ms: /usr/include/c++/11/algorithm (included 5535 times, avg 229 ms), included via:
> > > 987320 ms: /usr/include/c++/11/pstl/glue_algorithm_defs.h (included 5535 times, avg 178 ms), included via:
> > > ```
> > Do you have stats on how much this patch changes those numbers? 
> As mentioned since I couldn't remove the second rotate usage it doesn't change the top includes.
> I'm pretty sure I only touched hash_state cause it showed up in the top class parsing costs but now the impact is smaller. Maybe I looked at the numbers only for LLVMSupport back then.
Do you think this change is still worth doing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137918/new/

https://reviews.llvm.org/D137918



More information about the llvm-commits mailing list