llvm dev - Jun 2014 - [LLVMdev] Making it possible to clear the LLVMContext

Hello,

I'm trying to develop a way to reliably clean the LLVMContext in order to
make it possible to use it multiple times.

LLVMContext itself is an almost empty object delegating almost all its
content to LLVMContextImpl.
This makes it very clean ideally, because clearing the LLVMContext would be
as easy as deleting the LLVMContextImpl and creating a new one.

The problem is that for some reason which I'm not aware of LLVMContextImpl
is actually exposed as a public pointer in the LLVMContext interface,making
it publicly available to objects that use it directly (this seems to happen
quite a lot in the codebase).

In LLVMContext the LLVMContextImpl is contained in a pImpl pointer that is
const (the pointer itself can't be changed) and I guess this is some kind
of protection against object replacing the LLVMContextImpl directly, which
stops us from just deleting it + getting a new one.
In addition to that, being pImpl public, there is no guarantee that objects
don't rely on pImpl remaining always the same pointer.

This makes it more difficult to clear LLVMContext.

An approach I thought of could be adding a clear() method to LLVMContext
that:
 - Calls directly the destructor of LLVMContextImpl on the pImpl object
 - Uses a placement new to reinitialize the object.
 - Recreates the fixed metadata kinds like the LLVMContext constructor does

I'm attaching a patch that show this approach in this mail.

I would like to know a general idea about what people think about this and
see what people think would be the best approach would be.

Thanks,
Marcello
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140624/9c510069/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clear_llvm_context.patch
Type: application/octet-stream
Size: 2310 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140624/9c510069/attachment.obj>

What're the situation in which you need to clear it? If there are
outstanding users of it (given that you mention clients possibly
holding references to the pimpl, it sounds like you might have
outstanding users) then wouldn't they be at risk of breaking if you
mutate the LLVMContext underneath them?

& if you don't have outstanding users, is there any particular benefit
to resetting the LLVMContext compared to just making a new one?

On Tue, Jun 24, 2014 at 7:18 AM, Marcello Maggioni <hayarms at gmail.com>
wrote:
> Hello,
>
> I'm trying to develop a way to reliably clean the LLVMContext in order
to
> make it possible to use it multiple times.
>
> LLVMContext itself is an almost empty object delegating almost all its
> content to LLVMContextImpl.
> This makes it very clean ideally, because clearing the LLVMContext would be
> as easy as deleting the LLVMContextImpl and creating a new one.
>
> The problem is that for some reason which I'm not aware of
LLVMContextImpl
> is actually exposed as a public pointer in the LLVMContext interface,making
> it publicly available to objects that use it directly (this seems to happen
> quite a lot in the codebase).
>
> In LLVMContext the LLVMContextImpl is contained in a pImpl pointer that is
> const (the pointer itself can't be changed) and I guess this is some
kind of
> protection against object replacing the LLVMContextImpl directly, which
> stops us from just deleting it + getting a new one.
> In addition to that, being pImpl public, there is no guarantee that objects
> don't rely on pImpl remaining always the same pointer.
>
> This makes it more difficult to clear LLVMContext.
>
> An approach I thought of could be adding a clear() method to LLVMContext
> that:
>  - Calls directly the destructor of LLVMContextImpl on the pImpl object
>  - Uses a placement new to reinitialize the object.
>  - Recreates the fixed metadata kinds like the LLVMContext constructor does
>
> I'm attaching a patch that show this approach in this mail.
>
> I would like to know a general idea about what people think about this and
> see what people think would be the best approach would be.
>
> Thanks,
> Marcello
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

Hi,

On Tuesday, June 24, 2014 15:18:51 Marcello Maggioni
wrote:
> I'm trying to develop a way to reliably clean the LLVMContext in order
to
> make it possible to use it multiple times.
> 
> LLVMContext itself is an almost empty object delegating almost all its
> content to LLVMContextImpl.
> This makes it very clean ideally, because clearing the LLVMContext would be
> as easy as deleting the LLVMContextImpl and creating a new one.
> 
> The problem is that for some reason which I'm not aware of
LLVMContextImpl
> is actually exposed as a public pointer in the LLVMContext interface,making
> it publicly available to objects that use it directly (this seems to happen
> quite a lot in the codebase).
> 
> In LLVMContext the LLVMContextImpl is contained in a pImpl pointer that is
> const (the pointer itself can't be changed) and I guess this is some
kind
> of protection against object replacing the LLVMContextImpl directly, which
> stops us from just deleting it + getting a new one.
> In addition to that, being pImpl public, there is no guarantee that objects
> don't rely on pImpl remaining always the same pointer.
> 
> This makes it more difficult to clear LLVMContext.
I can't comment on your solution.
But again the open source OpenGL driver stack is suffering from problems
connected to
LLVMContext.

So our observation is, if we use several LLVMContexts within the program
execution
lifetime, LLVM accumulates memory that is only freed on destruction of the
managed singletons. It turns out that in SDNode::getValueTypeList(EVT) the non
simple
value types are cached and keyed by a llvm::Type value which in the end refers
to a
LLVMContext reference value. Which in turn leads to the llvm library hogging
memory
in this EVT std::set for each new LLVMContext.
For the OpenGL drivers using the global LLVMContext singleton leads to serious
threadsafeness problems. One driver is just non OpenGL conformant
non thread safe because of that, some others just tolerate LLVM hogging
memory because of that. Even worse for the ones just tolerating hogging
memory, I could potentially think of possible ABA like problems with a
new LLVMContext or llvm::Type being malloced at the same address than a previous
already deleted
one but not knowing about the cached EVT's which appear to belong to this
context.

That said I think you need to be careful since some of the llvm static caches
refer to
LLVMContext pointers and at least the one I mentioned above is not cleared if
either
a whole LLVMContext is cleared nor it is cleared by what you want to achieve.

Also for my interest:
Does anybody know of other statically cached object instances that are tied to
an
LLVMContext but not cleaned up when the referring LLVMContext is destroyed?

Greetings and thanks

Mathias

Hello,

the need here is to have a single LLVMContext used for multiple
compilations.

You make a good point about that by the way. If there are outstanding users
cleaning the context under their seats might still pose a risk to them, and
in that case deleting + newing a new LLVMContextImpl might actually not be
very different.

Marcello

2014-06-24 17:14 GMT+01:00 David Blaikie <dblaikie at gmail.com>:
> What're the situation in which you need to clear it? If there are
> outstanding users of it (given that you mention clients possibly
> holding references to the pimpl, it sounds like you might have
> outstanding users) then wouldn't they be at risk of breaking if you
> mutate the LLVMContext underneath them?
>
> & if you don't have outstanding users, is there any particular
benefit
> to resetting the LLVMContext compared to just making a new one?
>
> On Tue, Jun 24, 2014 at 7:18 AM, Marcello Maggioni <hayarms at
gmail.com>
> wrote:
> > Hello,
> >
> > I'm trying to develop a way to reliably clean the LLVMContext in
order to
> > make it possible to use it multiple times.
> >
> > LLVMContext itself is an almost empty object delegating almost all its
> > content to LLVMContextImpl.
> > This makes it very clean ideally, because clearing the LLVMContext
would
> be
> > as easy as deleting the LLVMContextImpl and creating a new one.
> >
> > The problem is that for some reason which I'm not aware of
> LLVMContextImpl
> > is actually exposed as a public pointer in the LLVMContext
> interface,making
> > it publicly available to objects that use it directly (this seems to
> happen
> > quite a lot in the codebase).
> >
> > In LLVMContext the LLVMContextImpl is contained in a pImpl pointer
that
> is
> > const (the pointer itself can't be changed) and I guess this is
some
> kind of
> > protection against object replacing the LLVMContextImpl directly,
which
> > stops us from just deleting it + getting a new one.
> > In addition to that, being pImpl public, there is no guarantee that
> objects
> > don't rely on pImpl remaining always the same pointer.
> >
> > This makes it more difficult to clear LLVMContext.
> >
> > An approach I thought of could be adding a clear() method to
LLVMContext
> > that:
> >  - Calls directly the destructor of LLVMContextImpl on the pImpl
object
> >  - Uses a placement new to reinitialize the object.
> >  - Recreates the fixed metadata kinds like the LLVMContext constructor
> does
> >
> > I'm attaching a patch that show this approach in this mail.
> >
> > I would like to know a general idea about what people think about this
> and
> > see what people think would be the best approach would be.
> >
> > Thanks,
> > Marcello
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140624/5ab1f1d5/attachment.html>

What you say here is pretty scary ... before saying anything about this I
think I will have a read in the source code.

If somebody in the meantime has any extra information about this it would
be useful :)

Marcello

2014-06-24 17:17 GMT+01:00 Mathias Fröhlich <Mathias.Froehlich at
gmx.net>:
>
> Hi,
>
> On Tuesday, June 24, 2014 15:18:51 Marcello Maggioni wrote:
> > I'm trying to develop a way to reliably clean the LLVMContext in
order to
> > make it possible to use it multiple times.
> >
> > LLVMContext itself is an almost empty object delegating almost all its
> > content to LLVMContextImpl.
> > This makes it very clean ideally, because clearing the LLVMContext
would
> be
> > as easy as deleting the LLVMContextImpl and creating a new one.
> >
> > The problem is that for some reason which I'm not aware of
> LLVMContextImpl
> > is actually exposed as a public pointer in the LLVMContext
> interface,making
> > it publicly available to objects that use it directly (this seems to
> happen
> > quite a lot in the codebase).
> >
> > In LLVMContext the LLVMContextImpl is contained in a pImpl pointer
that
> is
> > const (the pointer itself can't be changed) and I guess this is
some kind
> > of protection against object replacing the LLVMContextImpl directly,
> which
> > stops us from just deleting it + getting a new one.
> > In addition to that, being pImpl public, there is no guarantee that
> objects
> > don't rely on pImpl remaining always the same pointer.
> >
> > This makes it more difficult to clear LLVMContext.
>
> I can't comment on your solution.
> But again the open source OpenGL driver stack is suffering from problems
> connected to
> LLVMContext.
>
> So our observation is, if we use several LLVMContexts within the program
> execution
> lifetime, LLVM accumulates memory that is only freed on destruction of the
> managed singletons. It turns out that in SDNode::getValueTypeList(EVT) the
> non simple
> value types are cached and keyed by a llvm::Type value which in the end
> refers to a
> LLVMContext reference value. Which in turn leads to the llvm library
> hogging memory
> in this EVT std::set for each new LLVMContext.
> For the OpenGL drivers using the global LLVMContext singleton leads to
> serious
> threadsafeness problems. One driver is just non OpenGL conformant
> non thread safe because of that, some others just tolerate LLVM hogging
> memory because of that. Even worse for the ones just tolerating hogging
> memory, I could potentially think of possible ABA like problems with a
> new LLVMContext or llvm::Type being malloced at the same address than a
> previous already deleted
> one but not knowing about the cached EVT's which appear to belong to
this
> context.
>
> That said I think you need to be careful since some of the llvm static
> caches refer to
> LLVMContext pointers and at least the one I mentioned above is not cleared
> if either
> a whole LLVMContext is cleared nor it is cleared by what you want to
> achieve.
>
> Also for my interest:
> Does anybody know of other statically cached object instances that are
> tied to an
> LLVMContext but not cleaned up when the referring LLVMContext is destroyed?
>
> Greetings and thanks
>
> Mathias
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140624/498db0ea/attachment.html>

On Tue, Jun 24, 2014 at 9:17 AM, Mathias Fröhlich <Mathias.Froehlich at
gmx.net
> wrote:
>
> I can't comment on your solution.
> But again the open source OpenGL driver stack is suffering from problems
> connected to
> LLVMContext.
>
> So our observation is, if we use several LLVMContexts within the program
> execution
> lifetime, LLVM accumulates memory that is only freed on destruction of the
> managed singletons. It turns out that in SDNode::getValueTypeList(EVT) the
> non simple
> value types are cached and keyed by a llvm::Type value which in the end
> refers to a
> LLVMContext reference value. Which in turn leads to the llvm library
> hogging memory
> in this EVT std::set for each new LLVMContext.
> For the OpenGL drivers using the global LLVMContext singleton leads to
> serious
> threadsafeness problems. One driver is just non OpenGL conformant
> non thread safe because of that, some others just tolerate LLVM hogging
> memory because of that. Even worse for the ones just tolerating hogging
> memory, I could potentially think of possible ABA like problems with a
> new LLVMContext or llvm::Type being malloced at the same address than a
> previous already deleted
> one but not knowing about the cached EVT's which appear to belong to
this
> context.
>
It looks like this bug is in SDNode::getValueTypeList().  This is a real
bug that should be filed and fixed.

That said I think you need to be careful since some of the llvm
static
> caches refer to
> LLVMContext pointers and at least the one I mentioned above is not cleared
> if either
> a whole LLVMContext is cleared nor it is cleared by what you want to
> achieve.
>
> Also for my interest:
> Does anybody know of other statically cached object instances that are
> tied to an
> LLVMContext but not cleaned up when the referring LLVMContext is destroyed?
>
Not that I know of.  Any global that points to a Type* is a potential bug
and should probably be removed.

> Greetings and thanks
>
> Mathias
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140624/f0ae4e62/attachment.html>

Sent from my iPhone
> On Jun 24, 2014, at 12:17, Mathias Fröhlich <Mathias.Froehlich at
gmx.net> wrote:
> 
> 
> Hi,
> 
>> On Tuesday, June 24, 2014 15:18:51 Marcello Maggioni wrote:
>> I'm trying to develop a way to reliably clean the LLVMContext in
order to
>> make it possible to use it multiple times.
>> 
>> LLVMContext itself is an almost empty object delegating almost all its
>> content to LLVMContextImpl.
>> This makes it very clean ideally, because clearing the LLVMContext
would be
>> as easy as deleting the LLVMContextImpl and creating a new one.
>> 
>> The problem is that for some reason which I'm not aware of
LLVMContextImpl
>> is actually exposed as a public pointer in the LLVMContext
interface,making
>> it publicly available to objects that use it directly (this seems to
happen
>> quite a lot in the codebase).
>> 
>> In LLVMContext the LLVMContextImpl is contained in a pImpl pointer that
is
>> const (the pointer itself can't be changed) and I guess this is
some kind
>> of protection against object replacing the LLVMContextImpl directly,
which
>> stops us from just deleting it + getting a new one.
>> In addition to that, being pImpl public, there is no guarantee that
objects
>> don't rely on pImpl remaining always the same pointer.
>> 
>> This makes it more difficult to clear LLVMContext.
> 
> I can't comment on your solution.
> But again the open source OpenGL driver stack is suffering from problems
connected to
> LLVMContext.
> 
> So our observation is, if we use several LLVMContexts within the program
execution
> lifetime, LLVM accumulates memory that is only freed on destruction of the
> managed singletons. It turns out that in SDNode::getValueTypeList(EVT) the
non simple
> value types are cached and keyed by a llvm::Type value which in the end
refers to a
> LLVMContext reference value. Which in turn leads to the llvm library
hogging memory
> in this EVT std::set for each new LLVMContext.
> For the OpenGL drivers using the global LLVMContext singleton leads to
serious
> threadsafeness problems. One driver is just non OpenGL conformant
> non thread safe because of that, some others just tolerate LLVM hogging
> memory because of that. Even worse for the ones just tolerating hogging
> memory, I could potentially think of possible ABA like problems with a
> new LLVMContext or llvm::Type being malloced at the same address than a
previous already deleted
> one but not knowing about the cached EVT's which appear to belong to
this context.
> 
> That said I think you need to be careful since some of the llvm static
caches refer to
> LLVMContext pointers and at least the one I mentioned above is not cleared
if either
> a whole LLVMContext is cleared nor it is cleared by what you want to
achieve.
> 
> Also for my interest:
> Does anybody know of other statically cached object instances that are tied
to an
> LLVMContext but not cleaned up when the referring LLVMContext is destroyed?
> 
This is probably an independent bug, no? How about moving the evt cache to the
llvmcontext so that it goes away with it?
> Greetings and thanks
> 
> Mathias
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev