[PATCH] Add reference counting support to SourceMgr

Alp Toker alp at nuanti.com
Wed Jun 25 00:46:30 PDT 2014


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. 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




More information about the llvm-commits mailing list