[PATCH] D96035: [dsymutil][DWARFlinker] implement separate multi-thread processing for compile units.

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 5 16:51:28 PDT 2022


JDevlieghere added a comment.

I'm (still) very excited about this. I'm really happy that this lives behind a flag which addresses most of my original concerns. Making this the default in dsymutil would require a bunch of qualification but this approach allows me to do that gradually. One thing I suggested earlier I think was to run the lldb test suite with the new flag and see if that breaks anything. Did you ever get around to that? That should give us an initial level of confidence. Jim expressed some concerns about trying to squeeze everything in the same compilation unit. Greg, do you share that concern? Do we expect this will negatively impact lldb's performance? It would probably be worthwhile to do a few performance experiments here as well to make sure this doesn't regress debugging performance.

I'm also very excited that Sony is considering upstreaming `llvm-dva` (based on Paul's comment in D124082 <https://reviews.llvm.org/D124082>). In the past we've often stuck to binary compatibility because there's no great way to qualify big changes to the algorithm. But as long as all of this lives behind a flag that's not a dealbreaker.

If we're happy with the approach I think we should land this an iterate on smaller changes that will be much easier to review. One reason I've been so quiet in this review is that it takes a long time to page in everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96035



More information about the llvm-commits mailing list