ayokunle321 wrote:

> So it seems we are spending a lot of time calling `makeAbsolute`. Maybe we 
> can make `LoadedInputFiles` take base name, or a base name + file size pair 
> as key so we can avoid calling `makeAbsolute` on all the input files?

> Another direction to explore can be making the logic in 
> `buildLoadedInputFiles()` lazy. Currently, the amount of work it performs 
> depends on the size of the imported modules. I think it's usually better to 
> make it proportional to the size of the written module itself. (That holds 
> true especially for large module graphs.) Why don't we use the same 
> visitor+lookup pattern we already use for `HeaderFileInfo` here?

@jansvoboda11 @qiongsiwu

Looked into this. For the SLoc scan, I changed the `INPUT_FILE` record to carry 
the SLoc index and offset directly, captured in `WriteInputFiles` where we 
already walk the SLoc entries. So the reader reconstructs the location from 
those instead of scanning, which let me delete the per-module SLoc scan 
entirely. It adds ~322 B per PCM for my workload, which is minimal but is not 
nothing since we're wary about PCM size.  It does reduce the complexity and a 
net deletion in code overall.

For the outer lookup which asks "which loaded module holds the file", I keyed 
the map on size instead of the canonicalized path, so we skip 
`makeAbsolutePath` on every input file and only fall back to full path on the 
candidates in the same size bucket. I went with size alone because the string 
map inserts were ~20% of the added cost in @qiongsiwu's run, and a size key 
drops the string hashing entirely. Collisions should be rare, and full path 
still confirms identity, but let me know if you'd rather I key on name + size 
for the initial check. 

On a workload (generated my Claude) shaped like your SDK TU (212 modules, 
SDK-length paths) this removed ~77% of the added cost, which lines up with the 
~77% your instrumentation put on path canonicalization. So the eager build is 
cheap now even though it still walks the loaded graph.

On making `buildLoadedInputFiles()` actually lazy, from the visitor pattern it 
looks like each module would need to serialize its own input-file table into 
the PCM to probe instead of building the map at load time. I prototyped that 
and measured it and the table adds ~2 KB per PCM (~0.42%) to recover ~1.5% of 
the time regression (this is after the `INPUT_FILE` record change and the scan 
removal), and it's written to every PCM including leaf modules that nothing is 
ever written against. So I was hesitant to go ahead with it.

The O(written module) lazy version is definitely the better design and it would 
cut code complexity too. But the size-keying already made the eager build cheap 
enough that I'm not sure the table earns the ~2 KB (not sure what the threshold 
is for accepted PCM size increases).

My numbers are from synthetic workloads, so if @qiongsiwu could re-measure 
timing that would be great!

https://github.com/llvm/llvm-project/pull/209795
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to