Re: [Mono-dev] ObjectPool
We will be stress testing this. This seems to be the root cause of the other issues we have been experiencing, literally hundreds of things are broken because of this one little issue (anything with callbacks, concurrentstack/queue, etc etc). On Wed, Jul 25, 2012 at 7:43 AM, Jérémie Laval wrote: > There were indeed two bugs in there, a missing barrier and the issue you > described. > > I also commented out the code if you want more details. > > -- > Jérémie Laval > http://neteril.org > > > > On Wed, Jul 25, 2012 at 11:54 AM, Greg Young > wrote: >> >> We have had some moving forward in our mono stability issues. I >> figured I would start up a chat here about one place that seems to be >> causing many problems. >> >> >> >> https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Collections.Concurrent/ObjectPool.cs >> >> I am not really sure the code here is actually thread safe... Maybe it >> doesn't need to be for some reason? >> >> We have added volatiles for add/remove index (yuriy pointed out that >> threads were often stuck at line 86) but even with that im still not >> sure its actually threadsafe. There are some weird edge conditions >> that seem to be here (like if CompareExchange fails 3 times it will >> just never set the remove index and continue on). >> >> Somebody has obviously spent a lot of time thinking about this code >> and optimizing it. I want to make sure we get the full "why" behind >> things so we dont a) waste time b) introduce issues. >> >> Greg >> >> -- >> Le doute n'est pas une condition agréable, mais la certitude est absurde. >> ___ >> Mono-devel-list mailing list >> Mono-devel-list@lists.ximian.com >> http://lists.ximian.com/mailman/listinfo/mono-devel-list > > -- Le doute n'est pas une condition agréable, mais la certitude est absurde. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ObjectPool
Hi Jérémie, thank you for so quick fix! I'll build mono with your fix and let you know how it works. It seems that a lot of our previously reported problems with sockets are due to ObjectPool. -yuriy On Wed, Jul 25, 2012 at 2:43 PM, Jérémie Laval wrote: > There were indeed two bugs in there, a missing barrier and the issue you > described. > > I also commented out the code if you want more details. > > -- > Jérémie Laval > http://neteril.org > > > > On Wed, Jul 25, 2012 at 11:54 AM, Greg Young > wrote: >> >> We have had some moving forward in our mono stability issues. I >> figured I would start up a chat here about one place that seems to be >> causing many problems. >> >> >> >> https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Collections.Concurrent/ObjectPool.cs >> >> I am not really sure the code here is actually thread safe... Maybe it >> doesn't need to be for some reason? >> >> We have added volatiles for add/remove index (yuriy pointed out that >> threads were often stuck at line 86) but even with that im still not >> sure its actually threadsafe. There are some weird edge conditions >> that seem to be here (like if CompareExchange fails 3 times it will >> just never set the remove index and continue on). >> >> Somebody has obviously spent a lot of time thinking about this code >> and optimizing it. I want to make sure we get the full "why" behind >> things so we dont a) waste time b) introduce issues. >> >> Greg >> >> -- >> Le doute n'est pas une condition agréable, mais la certitude est absurde. >> ___ >> Mono-devel-list mailing list >> Mono-devel-list@lists.ximian.com >> http://lists.ximian.com/mailman/listinfo/mono-devel-list > > > > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > -- Yuriy Solodkyy (y.solod...@gmail.com) ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ObjectPool
There were indeed two bugs in there, a missing barrier and the issue you described. I also commented out the code if you want more details. -- Jérémie Laval http://neteril.org On Wed, Jul 25, 2012 at 11:54 AM, Greg Young wrote: > We have had some moving forward in our mono stability issues. I > figured I would start up a chat here about one place that seems to be > causing many problems. > > > > https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Collections.Concurrent/ObjectPool.cs > > I am not really sure the code here is actually thread safe... Maybe it > doesn't need to be for some reason? > > We have added volatiles for add/remove index (yuriy pointed out that > threads were often stuck at line 86) but even with that im still not > sure its actually threadsafe. There are some weird edge conditions > that seem to be here (like if CompareExchange fails 3 times it will > just never set the remove index and continue on). > > Somebody has obviously spent a lot of time thinking about this code > and optimizing it. I want to make sure we get the full "why" behind > things so we dont a) waste time b) introduce issues. > > Greg > > -- > Le doute n'est pas une condition agréable, mais la certitude est absurde. > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ObjectPool
On Wed, 2012-07-25 at 07:28 -0400, Greg Young wrote: > Yes we are using sgen. > > Good to know the history there as well. > > Cheers, > > Greg > Well, *actually* this is my guess ;) -- Regards, Konrad ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ObjectPool
Yes we are using sgen. Good to know the history there as well. Cheers, Greg On Wed, Jul 25, 2012 at 7:22 AM, Konrad Kruczyński wrote: > On Wed, 2012-07-25 at 07:17 -0400, Greg Young wrote: >> Yuriy will reply with more details but by just making Take return >> Creator() and nooping release many of our issues have gone away >> (including visible issues with concurrentstack). From what I have seen >> we also saw massive performance gains. >> > > Just for the curiosity: are you using SGEN? I've always thought that > ObjectPool in ConcurrentStack was used to reduce the pressure on the > garbage collector (Boehm, specifically). It is probably much less > important with generational collector like SGEN. > > -- > Regards, > Konrad > > -- Le doute n'est pas une condition agréable, mais la certitude est absurde. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ObjectPool
On Wed, 2012-07-25 at 07:17 -0400, Greg Young wrote: > Yuriy will reply with more details but by just making Take return > Creator() and nooping release many of our issues have gone away > (including visible issues with concurrentstack). From what I have seen > we also saw massive performance gains. > Just for the curiosity: are you using SGEN? I've always thought that ObjectPool in ConcurrentStack was used to reduce the pressure on the garbage collector (Boehm, specifically). It is probably much less important with generational collector like SGEN. -- Regards, Konrad ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ObjectPool
Yuriy will reply with more details but by just making Take return Creator() and nooping release many of our issues have gone away (including visible issues with concurrentstack). From what I have seen we also saw massive performance gains. On Wed, Jul 25, 2012 at 6:54 AM, Greg Young wrote: > We have had some moving forward in our mono stability issues. I > figured I would start up a chat here about one place that seems to be > causing many problems. > > > https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Collections.Concurrent/ObjectPool.cs > > I am not really sure the code here is actually thread safe... Maybe it > doesn't need to be for some reason? > > We have added volatiles for add/remove index (yuriy pointed out that > threads were often stuck at line 86) but even with that im still not > sure its actually threadsafe. There are some weird edge conditions > that seem to be here (like if CompareExchange fails 3 times it will > just never set the remove index and continue on). > > Somebody has obviously spent a lot of time thinking about this code > and optimizing it. I want to make sure we get the full "why" behind > things so we dont a) waste time b) introduce issues. > > Greg > > -- > Le doute n'est pas une condition agréable, mais la certitude est absurde. -- Le doute n'est pas une condition agréable, mais la certitude est absurde. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-dev] ObjectPool
We have had some moving forward in our mono stability issues. I figured I would start up a chat here about one place that seems to be causing many problems. https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Collections.Concurrent/ObjectPool.cs I am not really sure the code here is actually thread safe... Maybe it doesn't need to be for some reason? We have added volatiles for add/remove index (yuriy pointed out that threads were often stuck at line 86) but even with that im still not sure its actually threadsafe. There are some weird edge conditions that seem to be here (like if CompareExchange fails 3 times it will just never set the remove index and continue on). Somebody has obviously spent a lot of time thinking about this code and optimizing it. I want to make sure we get the full "why" behind things so we dont a) waste time b) introduce issues. Greg -- Le doute n'est pas une condition agréable, mais la certitude est absurde. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
