[libcxx-commits] [PATCH] D144322: [libc++][Modules] Make top level modules for all C++ headers with OS/clang versions

[Duncan P. N. Exon Smith via Phabricator via libcxx-commits]

dexonsmith added a comment.

In D144322#4275378 <https://reviews.llvm.org/D144322#4275378>, @iana wrote:

> In D144322#4275247 <https://reviews.llvm.org/D144322#4275247>, @dexonsmith wrote:
>
>> I'd be curious to see just how bad the perf is with implicitly-discovered-and-explicitly-built modules. Maybe it wouldn't be as bad as suspected. And will get faster with the scanning speedups @jansvoboda11 is working on.
>>
>> - Do we know how deep the build graph would be? (Would we get good parallelism when building?)
>
> I made a rough graph that's not quite accurate but is probably close enough to answer that question. It's way more wide and shallow than I thought it would be. 
> F27171262: libcxx-dependencies.gv.pdf <https://reviews.llvm.org/F27171262>
> It's not so much parallelism that we're worried about, it's more like launching clang ~200 times to build the algorithm module and later opening 200 pcm files.

In the fullness of time, we should have some sort of compile server rather than launching 200 processes. Maybe opening 200 PCM files would be slow; maybe we can optimize that too. (Perhaps once other pieces are in place that allow it to be efficient, we can/should move toward this solution, since it seems like the build graph supports it.)

>> - I doubt lookups will be too slow (but could be wrong...). I don't remember X (X=5? X=12?), but there's a new top-level lookup table created in a module if it depends on X+ other modules transitively that don't have a top-level lookup table. @rsmith implemented this to unblock using top-level modules for fine-grained headers in C headers, which sounds pretty similar to this situation.
>>
>>> 1. Make top level modules for all the public headers, and programmatically generate the minimum number of top level modules for the private detail headers at build time such that there are no cycles. I'm still working on this one.
>>
>> A variant on this: put every header in its own *sub*module. It's probably not hard (and independently useful) to have clang (optionally) diagnose cycles between submodules; this way you can ensure cycles don't accidentally get added over time, without needing a separate top-level module for each header. WDYT?
>
> That doesn't work, clang doesn't care if it can interleave the submodules, i.e. it's not mad about algorithm.__algorithm and chrono.__chrono, it cares that anything in algorithm is cyclic with anything in chrono.

I don't think we disagree.

To clarify, I imagine it would not be hard to write a patch that taught clang how to diagnose this (flag could be `-Wsubmodule-cycles` or something). This could help users (like libc++ devs) that want to avoid accidentally adding cycles between submodules. Similarly, we could have a `-Wheader-cycles` flag; not sure if that is more or less useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144322