[llvm-dev] LLVMContext: Threads and Ownership.

Sun Sep 16 16:55:33 PDT 2018

In the most basic case, I'd imagine something like this:

auto C = std::make_shared<LLVMContext>();
struct ModuleAndSharedContextDeleter { std::shared_ptr<LLVMContext> C;
operator()(Module *M) { delete M; } /* ctor to init C */};
std::unique_ptr<Module, ModuleAndSharedDeleter> M(new Module(C.get()),
ModuleAndSharedContextDeleter(C));

(or invert this and traffic in structs that have a unique_ptr<Module> and a
shared_ptr<LLVMContext> as members - though the above example has the
advantage that it mostly looks like unique_ptr, and you can change
ownership (pass the unique_ptr<Module, CustomDeleter> to a shared_ptr and
it'll correctly propagate the deleter, etc))
...

But it sounds like you're concerned with a situation in which there are a
wide variety of things making Modules that are maybe outside the purview of
Orc but need this ownership model? That would seem unfortunate & I'm not
quite sure I'm picturing the situation you have in mind.

On Sun, Sep 16, 2018 at 4:27 PM Lang Hames <lhames at gmail.com> wrote:

> I'd think/suggest ref-counted LLVMContext ownership would be done by
>> wrapping/external tracking in this use case.
>
>
> What kind of wrapping are you imagining? I'm worried that maintaining an
> external context ref-count is going to be awkward and error prone, but
> perhaps there are idioms that would make this easier than I am imagining.
>
> I think it may be simpler and safer to expose the number of Modules
> managed by an LLVMContext. That way the "ref-count" always reflects the
> number of modules using the context, and tracking can be set up and managed
> entirely from the LLVMContext creation site (by adding the context to an
> 'automatically managed' set), rather than intruding to every Module
> creation point.
>
> We would still need to add a mutex to LLVMContext to make this thread safe
> though.
>
> -- Lang.
>
> On Sun, Sep 16, 2018 at 10:22 AM David Blaikie <dblaikie at gmail.com> wrote:
>
>> Agreed, the existing ownership seems sub-optimal. I wouldn't say broken,
>> but subtle at least - looks like you get the choice to either manage the
>> ownership of the Module object yourself, or let the context handle it (eg:
>> currently it'd be valid to just do "{ LLVMContext C; new Module(C); new
>> Module(C); }" - Modules end up owned by the context and cleaned up there).
>>
>> Might be hard to migrate existing users away from this without silently
>> introducing memory leaks... maybe with some significant API breakage - move
>> Module construction to a factory/helper that returns a
>> std::unique_ptr<Module> - requiring every Module construction to be
>> revisited, and users relying on LLVMContext based ownership/cleanup to
>> redesign their code.
>>
>> As to the original question - gut reaction: this doesn't seem like
>> something that's general-purpose enough to be implemented in the
>> LLVMContext/Module itself. I think a reasonable ownership model for
>> LLVMContext/Module is that the user is required to ensure the LLVMContext
>> outlives all Modules created within it (same way a user of std::vector is
>> required to ensure that the vector is not reallocated so long as they're
>> keeping pointers/references to elements in it). I'd think/suggest
>> ref-counted LLVMContext ownership would be done by wrapping/external
>> tracking in this use case.
>>
>> - Dave
>>
>> On Sat, Sep 15, 2018 at 9:30 PM Lang Hames via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Actually, looking at the destructors for LLVMContext and Module I do not
>>> think the current ownership scheme makes sense, so this might be a good
>>> opportunity to re-think it.
>>>
>>> Right now an LLVMContext owns a list of modules (see
>>> LLVMContextImpl::OwnedModules) that it destroys when its destructor is
>>> called.  Modules remove themselves from this list if they are destructed
>>> before the context:
>>>
>>> Module::~Module() {
>>>   Context.removeModule(this);
>>>   ...
>>>
>>> LLVMContextImpl::~LLVMContextImpl() {
>>>   // NOTE: We need to delete the contents of OwnedModules, but Module's
>>> dtor
>>>   // will call LLVMContextImpl::removeModule, thus invalidating
>>> iterators into
>>>   // the container. Avoid iterators during this operation:
>>>   while (!OwnedModules.empty())
>>>     delete *OwnedModules.begin();
>>>     ...
>>>
>>> This makes it unsafe to hold a unique_ptr to a Module: If any Module is
>>> still alive when its context goes out of scope it will be double freed,
>>> first by the LLVMContextImpl destructor and then again by the unique ptr.
>>> Idiomatic scoping means that we tend not to see this in practice  (Module
>>> takes an LLVMContext reference, meaning we always declare the context
>>> first, so it goes out of scope last), but makes the context ownership
>>> redundant: the modules are always freed first via their unique_ptr's.
>>>
>>> I don't think it makes sense for LLVMContext to own Modules. I think
>>> that Modules should share ownership of their LLVMContext via a shared_ptr.
>>>
>>> Thoughts?
>>>
>>> Cheers,
>>> Lang.
>>>
>>> On Sat, Sep 15, 2018 at 4:14 PM Lang Hames <lhames at gmail.com> wrote:
>>>
>>>> Hi All,
>>>>
>>>> ORC's new concurrent compilation model generates some interesting
>>>> lifetime and thread safety questions around LLVMContext: We need multiple
>>>> LLVMContexts (one per module in the simplest case, but at least one per
>>>> thread), and the lifetime of each context depends on the execution path of
>>>> the JIT'd code. We would like to deallocate contexts once all modules
>>>> associated with them have been compiled, but there is no safe or easy way
>>>> to check that condition at the moment as LLVMContext does not expose how
>>>> many modules are associated with it.
>>>>
>>>> One way to fix this would be to add a mutex to LLVMContext, and expose
>>>> this and the module count. Then in the IR-compiling layer of the JIT we
>>>> could have something like:
>>>>
>>>> // Compile finished, time to deallocate the module.
>>>> // Explicit deletes used for clarity, we would use unique_ptrs in
>>>> practice.
>>>> auto &Ctx = Mod->getContext();
>>>> delete Mod;
>>>> std::lock_guard<std::mutex> Lock(Ctx->getMutex());
>>>> if (Ctx.getNumModules())
>>>>   delete Ctx;
>>>>
>>>> Another option would be to invert the ownership model and say that each
>>>> Module shares ownership of its LLVMContext. That way LLVMContexts would be
>>>> automatically deallocated when the last module using them is destructed
>>>> (providing no other shared_ptrs to the context are held elsewhere).
>>>>
>>>> There are other possible approaches (e.g. side tables for the mutex and
>>>> module count) but before I spent too much time on it I wanted to see
>>>> whether anyone else has encountered these issues or has opinions on
>>>> solutions.
>>>>
>>>> Cheers,
>>>> Lang.
>>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180916/7f21bf6e/attachment.html>