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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D96035#3285620 <https://reviews.llvm.org/D96035#3285620>, @clayborg wrote:

> In D96035#3284921 <https://reviews.llvm.org/D96035#3284921>, @avl wrote:
>
>> In D96035#3284832 <https://reviews.llvm.org/D96035#3284832>, @dblaikie wrote:
>>
>>> In D96035#3284247 <https://reviews.llvm.org/D96035#3284247>, @avl wrote:
>>>
>>>> rebased.
>>>> refactored.
>>>> implemented performance improvements(it is 1.3x slower in single thread mode, 2.4x faster im multi-thread mode).
>>>
>>> 1.3x slower means what took 1second before now takes 2.3 seconds?
>>
>> No, sorry, I meant  what took 1second before now takes 1*1.3 = 1.3sec.
>>
>>>> implemented types splitting.
>>>
>>> This means that anonymous/non-linkage types are correctly kept in their original CUs?
>>
>> yes. anonymous/non-linkage types are correctly kept in their original CUs. These are tests for that functionality:
>>
>> odr-nested-types1.test
>> odr-types-in-subprogram1.test
>>
>>> Does it mean other things too/instead? (splitting up types into chunks to avoid exceptionally large units containing all types?)
>>
>> splitting up types into chunks is not implemented yet. I think it would take some time. I would prefer to implement it as an separate improvement for this patch.
>
> I do believe that splitting types up into a compile unit that matches the DW_AT_decl_file would make this patch really hard to resist as it then makes the DWARF the best it can be.

I think the major reason it'd be resisted would be the cost - increasing dsymutil time significantly would be a hard sell. I think it's pretty understandable that the sort of parallel processing @avl's experimenting with here wouldn't be able to do that efficiently.

Arbitrarily grouping types into smallish buckets to reduce the overhead for DWARF consumers seems more feasible, though, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96035