[PATCH] Add reference counting support to SourceMgr

[David Blaikie]

On Wed, Jun 25, 2014 at 12:46 AM, Alp Toker <alp at nuanti.com> wrote:
>
> On 25/06/2014 08:39, David Blaikie wrote:
>>
>> Not sure where is best to have the discussion, but I'd like to
>> understand why this refcounting needs to be /intrusive/. It always
>> seems like a wart if code is taking ownership without documenting that
>> contract through the type system (and this is case where intrusive
>> reference counting is required/used - a raw pointer is passed around
>> and some subsystem takes ownership of that, while sharing it with some
>> other, unknown, owner).
>
>
> That's the theory, but in practice we're just passing around or storing a
> MemoryBuffer/bool pair in various parts of clang, and doing it
> inconsistently. The moment we hit a function without a
> DoesNotFree/Owns/DontRelease/CallerOwns/FreeAtEnd parameter, we end up
> copying the memory buffer as a workaround.
>
> Intrusive reference counting encapsulated within SourceMgr/SourceManager
> gives us a chance to fix all the known memory issues in one shot without a
> lot of surgery and without exposing the ownership detail to consumers, which
> is cool.

This, I think, is where we disagree. I don't like hiding/pretending
that that ownership isn't what it is. It makes clients
subtle/confusing.

> I think it actually models and optimizes what the code is trying to
> do right now, even if it doesn't model what we ultimately want:
>
> Some day it'll be great to introduce a centralised source manager that's
> smart enough not only to handle storage, but also to unique requests for
> memory mapped files and manage OS resources responsibly, as well as
> coordinating with the stat cache. But that's some way off.
>
> Alp.
>
>
>>
>> - David
>>
>> On Tue, Jun 24, 2014 at 10:27 PM, Alp Toker <alp at nuanti.com> wrote:
>>>
>>> The attached patch switches SourceMgr to use reference counting
>>> internally
>>> by making MemoryBuffer optionally refcounted.
>>>
>>> With this change it becomes possible to share ownership of memory buffers
>>> between multiple source managers in the backend and frontend instead of
>>> copying file contents into memory or re-reading from disk repeatedly.
>>>
>>> These are supporting changes for clang with no functional change on the
>>> LLVM
>>> side.
>>>
>>> See thread "[PATCH] Don't duplicate entire file contents in memory"
>>> posted
>>> to cfe-commits for details.
>>>
>>> Alp.
>>>
>>> --
>>> http://www.nuanti.com
>>> the browser experts
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>
> --
> http://www.nuanti.com
> the browser experts
>