[PATCH] Add a lock() function in PassRegistry to speed up multi-thread synchronization.

Chandler Carruth chandlerc at google.com
Wed Mar 4 16:54:01 PST 2015


Sorry it has taken some time to reply, but I firmly disagree with this
patch. I do not think this is the correct direction for LLVM. It makes an
already confusing and poorly understood API easy to use in a buggy way by
becoming modal in its concurrency. I think that is the wrong design.

On Fri, Feb 27, 2015 at 7:21 AM, Erik Eckstein <eeckstein at apple.com> wrote:

> Hi Owen, Chandler,
>
> > Do you mean with a release *+* asserts build?
>
>
> Yes, sorry I wrote it wrong.
>
> I think initial description was not very good. I'll try to explailn again:
> there are two issues
>
> 1. multithread-performance problem in the assert build: I understand that
> assert builds are slower than release builds, but in this case it's a
> factor of 4 when running with 4 threads, i.e. the multithreaded version is
> slower than the single-threaded version! This makes the asserts build
> useless for my purpose.
>

I would much rather remove the assert than add this complexity. I think
making the synchronization contract of an API modal and subtle in this way
is a terrible tradeoff to fix a performance problem in an *asserts* build.


>
> 2. Even in the release build there is a performance penalty of about 5%
> because of the mutex.
>

You mean without asserts at all? I cannot reproduce this and I have tried
very hard. As far as I'm aware, the only way this happens is if you
continually destroy and re-create your passmanager. If you stop doing that,
the performance hit should go away.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150304/d200e6d4/attachment.html>


More information about the llvm-commits mailing list