You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2023/08/01 02:32:04 UTC
Re: [pool] 2.12.0 update
OK, I found the source of the performance hit. In the POOL-411 changes, we
had inadvertently forced every register to acquire a write lock from the
keylock. I think I also finally definitively fixed the root issue there.
The tricky bit about the numInterested tracking is that the counters are
attached to the ObjectDeques, which can be replaced. If this happens while
waiting for a write lock in either register or deregister, the code can end
up updating the counter on a pool that has been replaced. I added checks
to trap deregistration of a null pool (should never happen) and followed
Sebb's suggestion to add a check for numInterested going negative. The
accounting setup is very efficient, but tricky to maintain. For 3.0, we
might consider moving numInterested tracking to a hashmap. For 2.x, I
think the setup is fixed now and performance is the same as earlier
versions. Soak tests look good.
One last thing I would like to do before we cut 2.12.0:
We are going to be making incompatible changes in 3.0 and I don't think we
need to telegraph all of the API changes via deprecations in 2.x - most
notably the recent method name changes of the form s/Time/Duration. I
understand the rationale for these changes, but they make the 2.x code very
messy with double deprecations - first from the "millis" methods and then
from "Time" to "Duration." I think it would be better to keep the existing
deprecations for the "millis" methods, but drop the new "Duration" ones and
remove deprecations for the ones they replace. I can see the argument that
it is better to tell users now, but that takes away flexibility in 3.0 and
makes the API look very confusing with so many methods that do the same
thing. Any objections ?
Phil
On Sat, Jul 29, 2023 at 3:59 PM Phil Steitz <ph...@gmail.com> wrote:
> I have run my first round of soak and performance tests on what is now in
> the 2.x branch. Good news is the code looks stable. Not so good news is
> it appears that GKOP performance has taken a material hit vs 2.11 and
> earlier versions. I need to confirm this via more targeted tests and if it
> turns out not to be real, figure out what is causing it. Hopefully I will
> get to this done in the next few days.
>
> Phil
>
Re: [pool] 2.12.0 update
Posted by Gary Gregory <ga...@gmail.com>.
It's all good Phil :-) I'm glad you took the time to look things over.
Gary
On Sun, Sep 10, 2023, 5:18 PM Phil Steitz <ph...@gmail.com> wrote:
> Well, now I really do need to apologize. I looked carefully again at the
> japicmp report and now agree that it is best to leave things as they are.
> I don't think it is likely we will change the names again and this is what
> deprecations are for. So as far as I am concerned, things are good to go.
> I will continue testing (including with the new DBCP release) and encourage
> others to review the code and test their apps as well; but I don't think
> any more changes are needed to the code in the 2_X branch now.
>
> Phil
>
> On Sat, Sep 9, 2023 at 3:40 PM Gary Gregory <ga...@gmail.com>
> wrote:
>
> > Thanks for the update, no need to apologize :-)
> >
> > Gary
> >
> > On Sat, Sep 9, 2023, 6:31 PM Phil Steitz <ph...@gmail.com> wrote:
> >
> > > Sorry I got busy. I will they to get final changes in tomorrow or
> > > convince myself it is ok to release without them. Apologies for the
> > delay
> > >
> > > > On Sep 9, 2023, at 6:41 AM, Gary Gregory <ga...@gmail.com>
> > wrote:
> > > >
> > > > Hi Phil,
> > > >
> > > > Where are we on a 2.12.0 release candidate?
> > > >
> > > > Gary
> > > >
> > > >> On Mon, Jul 31, 2023 at 10:33 PM Phil Steitz <phil.steitz@gmail.com
> >
> > > wrote:
> > > >>
> > > >> OK, I found the source of the performance hit. In the POOL-411
> > > changes, we
> > > >> had inadvertently forced every register to acquire a write lock from
> > the
> > > >> keylock. I think I also finally definitively fixed the root issue
> > > there.
> > > >> The tricky bit about the numInterested tracking is that the counters
> > are
> > > >> attached to the ObjectDeques, which can be replaced. If this
> happens
> > > while
> > > >> waiting for a write lock in either register or deregister, the code
> > can
> > > end
> > > >> up updating the counter on a pool that has been replaced. I added
> > > checks
> > > >> to trap deregistration of a null pool (should never happen) and
> > followed
> > > >> Sebb's suggestion to add a check for numInterested going negative.
> > The
> > > >> accounting setup is very efficient, but tricky to maintain. For
> 3.0,
> > we
> > > >> might consider moving numInterested tracking to a hashmap. For
> 2.x, I
> > > >> think the setup is fixed now and performance is the same as earlier
> > > >> versions. Soak tests look good.
> > > >>
> > > >> One last thing I would like to do before we cut 2.12.0:
> > > >>
> > > >> We are going to be making incompatible changes in 3.0 and I don't
> > think
> > > we
> > > >> need to telegraph all of the API changes via deprecations in 2.x -
> > most
> > > >> notably the recent method name changes of the form
> s/Time/Duration. I
> > > >> understand the rationale for these changes, but they make the 2.x
> code
> > > very
> > > >> messy with double deprecations - first from the "millis" methods and
> > > then
> > > >> from "Time" to "Duration." I think it would be better to keep the
> > > existing
> > > >> deprecations for the "millis" methods, but drop the new "Duration"
> > ones
> > > and
> > > >> remove deprecations for the ones they replace. I can see the
> argument
> > > that
> > > >> it is better to tell users now, but that takes away flexibility in
> 3.0
> > > and
> > > >> makes the API look very confusing with so many methods that do the
> > same
> > > >> thing. Any objections ?
> > > >>
> > > >> Phil
> > > >>
> > > >>> On Sat, Jul 29, 2023 at 3:59 PM Phil Steitz <phil.steitz@gmail.com
> >
> > > wrote:
> > > >>>
> > > >>> I have run my first round of soak and performance tests on what is
> > now
> > > in
> > > >>> the 2.x branch. Good news is the code looks stable. Not so good
> > news
> > > is
> > > >>> it appears that GKOP performance has taken a material hit vs 2.11
> and
> > > >>> earlier versions. I need to confirm this via more targeted tests
> and
> > > if it
> > > >>> turns out not to be real, figure out what is causing it.
> Hopefully I
> > > will
> > > >>> get to this done in the next few days.
> > > >>>
> > > >>> Phil
> > > >>>
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > > For additional commands, e-mail: dev-help@commons.apache.org
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> >
>
Re: [pool] 2.12.0 update
Posted by Phil Steitz <ph...@gmail.com>.
Well, now I really do need to apologize. I looked carefully again at the
japicmp report and now agree that it is best to leave things as they are.
I don't think it is likely we will change the names again and this is what
deprecations are for. So as far as I am concerned, things are good to go.
I will continue testing (including with the new DBCP release) and encourage
others to review the code and test their apps as well; but I don't think
any more changes are needed to the code in the 2_X branch now.
Phil
On Sat, Sep 9, 2023 at 3:40 PM Gary Gregory <ga...@gmail.com> wrote:
> Thanks for the update, no need to apologize :-)
>
> Gary
>
> On Sat, Sep 9, 2023, 6:31 PM Phil Steitz <ph...@gmail.com> wrote:
>
> > Sorry I got busy. I will they to get final changes in tomorrow or
> > convince myself it is ok to release without them. Apologies for the
> delay
> >
> > > On Sep 9, 2023, at 6:41 AM, Gary Gregory <ga...@gmail.com>
> wrote:
> > >
> > > Hi Phil,
> > >
> > > Where are we on a 2.12.0 release candidate?
> > >
> > > Gary
> > >
> > >> On Mon, Jul 31, 2023 at 10:33 PM Phil Steitz <ph...@gmail.com>
> > wrote:
> > >>
> > >> OK, I found the source of the performance hit. In the POOL-411
> > changes, we
> > >> had inadvertently forced every register to acquire a write lock from
> the
> > >> keylock. I think I also finally definitively fixed the root issue
> > there.
> > >> The tricky bit about the numInterested tracking is that the counters
> are
> > >> attached to the ObjectDeques, which can be replaced. If this happens
> > while
> > >> waiting for a write lock in either register or deregister, the code
> can
> > end
> > >> up updating the counter on a pool that has been replaced. I added
> > checks
> > >> to trap deregistration of a null pool (should never happen) and
> followed
> > >> Sebb's suggestion to add a check for numInterested going negative.
> The
> > >> accounting setup is very efficient, but tricky to maintain. For 3.0,
> we
> > >> might consider moving numInterested tracking to a hashmap. For 2.x, I
> > >> think the setup is fixed now and performance is the same as earlier
> > >> versions. Soak tests look good.
> > >>
> > >> One last thing I would like to do before we cut 2.12.0:
> > >>
> > >> We are going to be making incompatible changes in 3.0 and I don't
> think
> > we
> > >> need to telegraph all of the API changes via deprecations in 2.x -
> most
> > >> notably the recent method name changes of the form s/Time/Duration. I
> > >> understand the rationale for these changes, but they make the 2.x code
> > very
> > >> messy with double deprecations - first from the "millis" methods and
> > then
> > >> from "Time" to "Duration." I think it would be better to keep the
> > existing
> > >> deprecations for the "millis" methods, but drop the new "Duration"
> ones
> > and
> > >> remove deprecations for the ones they replace. I can see the argument
> > that
> > >> it is better to tell users now, but that takes away flexibility in 3.0
> > and
> > >> makes the API look very confusing with so many methods that do the
> same
> > >> thing. Any objections ?
> > >>
> > >> Phil
> > >>
> > >>> On Sat, Jul 29, 2023 at 3:59 PM Phil Steitz <ph...@gmail.com>
> > wrote:
> > >>>
> > >>> I have run my first round of soak and performance tests on what is
> now
> > in
> > >>> the 2.x branch. Good news is the code looks stable. Not so good
> news
> > is
> > >>> it appears that GKOP performance has taken a material hit vs 2.11 and
> > >>> earlier versions. I need to confirm this via more targeted tests and
> > if it
> > >>> turns out not to be real, figure out what is causing it. Hopefully I
> > will
> > >>> get to this done in the next few days.
> > >>>
> > >>> Phil
> > >>>
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
Re: [pool] 2.12.0 update
Posted by Gary Gregory <ga...@gmail.com>.
Thanks for the update, no need to apologize :-)
Gary
On Sat, Sep 9, 2023, 6:31 PM Phil Steitz <ph...@gmail.com> wrote:
> Sorry I got busy. I will they to get final changes in tomorrow or
> convince myself it is ok to release without them. Apologies for the delay
>
> > On Sep 9, 2023, at 6:41 AM, Gary Gregory <ga...@gmail.com> wrote:
> >
> > Hi Phil,
> >
> > Where are we on a 2.12.0 release candidate?
> >
> > Gary
> >
> >> On Mon, Jul 31, 2023 at 10:33 PM Phil Steitz <ph...@gmail.com>
> wrote:
> >>
> >> OK, I found the source of the performance hit. In the POOL-411
> changes, we
> >> had inadvertently forced every register to acquire a write lock from the
> >> keylock. I think I also finally definitively fixed the root issue
> there.
> >> The tricky bit about the numInterested tracking is that the counters are
> >> attached to the ObjectDeques, which can be replaced. If this happens
> while
> >> waiting for a write lock in either register or deregister, the code can
> end
> >> up updating the counter on a pool that has been replaced. I added
> checks
> >> to trap deregistration of a null pool (should never happen) and followed
> >> Sebb's suggestion to add a check for numInterested going negative. The
> >> accounting setup is very efficient, but tricky to maintain. For 3.0, we
> >> might consider moving numInterested tracking to a hashmap. For 2.x, I
> >> think the setup is fixed now and performance is the same as earlier
> >> versions. Soak tests look good.
> >>
> >> One last thing I would like to do before we cut 2.12.0:
> >>
> >> We are going to be making incompatible changes in 3.0 and I don't think
> we
> >> need to telegraph all of the API changes via deprecations in 2.x - most
> >> notably the recent method name changes of the form s/Time/Duration. I
> >> understand the rationale for these changes, but they make the 2.x code
> very
> >> messy with double deprecations - first from the "millis" methods and
> then
> >> from "Time" to "Duration." I think it would be better to keep the
> existing
> >> deprecations for the "millis" methods, but drop the new "Duration" ones
> and
> >> remove deprecations for the ones they replace. I can see the argument
> that
> >> it is better to tell users now, but that takes away flexibility in 3.0
> and
> >> makes the API look very confusing with so many methods that do the same
> >> thing. Any objections ?
> >>
> >> Phil
> >>
> >>> On Sat, Jul 29, 2023 at 3:59 PM Phil Steitz <ph...@gmail.com>
> wrote:
> >>>
> >>> I have run my first round of soak and performance tests on what is now
> in
> >>> the 2.x branch. Good news is the code looks stable. Not so good news
> is
> >>> it appears that GKOP performance has taken a material hit vs 2.11 and
> >>> earlier versions. I need to confirm this via more targeted tests and
> if it
> >>> turns out not to be real, figure out what is causing it. Hopefully I
> will
> >>> get to this done in the next few days.
> >>>
> >>> Phil
> >>>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [pool] 2.12.0 update
Posted by Phil Steitz <ph...@gmail.com>.
Sorry I got busy. I will they to get final changes in tomorrow or convince myself it is ok to release without them. Apologies for the delay
> On Sep 9, 2023, at 6:41 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi Phil,
>
> Where are we on a 2.12.0 release candidate?
>
> Gary
>
>> On Mon, Jul 31, 2023 at 10:33 PM Phil Steitz <ph...@gmail.com> wrote:
>>
>> OK, I found the source of the performance hit. In the POOL-411 changes, we
>> had inadvertently forced every register to acquire a write lock from the
>> keylock. I think I also finally definitively fixed the root issue there.
>> The tricky bit about the numInterested tracking is that the counters are
>> attached to the ObjectDeques, which can be replaced. If this happens while
>> waiting for a write lock in either register or deregister, the code can end
>> up updating the counter on a pool that has been replaced. I added checks
>> to trap deregistration of a null pool (should never happen) and followed
>> Sebb's suggestion to add a check for numInterested going negative. The
>> accounting setup is very efficient, but tricky to maintain. For 3.0, we
>> might consider moving numInterested tracking to a hashmap. For 2.x, I
>> think the setup is fixed now and performance is the same as earlier
>> versions. Soak tests look good.
>>
>> One last thing I would like to do before we cut 2.12.0:
>>
>> We are going to be making incompatible changes in 3.0 and I don't think we
>> need to telegraph all of the API changes via deprecations in 2.x - most
>> notably the recent method name changes of the form s/Time/Duration. I
>> understand the rationale for these changes, but they make the 2.x code very
>> messy with double deprecations - first from the "millis" methods and then
>> from "Time" to "Duration." I think it would be better to keep the existing
>> deprecations for the "millis" methods, but drop the new "Duration" ones and
>> remove deprecations for the ones they replace. I can see the argument that
>> it is better to tell users now, but that takes away flexibility in 3.0 and
>> makes the API look very confusing with so many methods that do the same
>> thing. Any objections ?
>>
>> Phil
>>
>>> On Sat, Jul 29, 2023 at 3:59 PM Phil Steitz <ph...@gmail.com> wrote:
>>>
>>> I have run my first round of soak and performance tests on what is now in
>>> the 2.x branch. Good news is the code looks stable. Not so good news is
>>> it appears that GKOP performance has taken a material hit vs 2.11 and
>>> earlier versions. I need to confirm this via more targeted tests and if it
>>> turns out not to be real, figure out what is causing it. Hopefully I will
>>> get to this done in the next few days.
>>>
>>> Phil
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [pool] 2.12.0 update
Posted by Gary Gregory <ga...@gmail.com>.
Hi Phil,
Where are we on a 2.12.0 release candidate?
Gary
On Mon, Jul 31, 2023 at 10:33 PM Phil Steitz <ph...@gmail.com> wrote:
>
> OK, I found the source of the performance hit. In the POOL-411 changes, we
> had inadvertently forced every register to acquire a write lock from the
> keylock. I think I also finally definitively fixed the root issue there.
> The tricky bit about the numInterested tracking is that the counters are
> attached to the ObjectDeques, which can be replaced. If this happens while
> waiting for a write lock in either register or deregister, the code can end
> up updating the counter on a pool that has been replaced. I added checks
> to trap deregistration of a null pool (should never happen) and followed
> Sebb's suggestion to add a check for numInterested going negative. The
> accounting setup is very efficient, but tricky to maintain. For 3.0, we
> might consider moving numInterested tracking to a hashmap. For 2.x, I
> think the setup is fixed now and performance is the same as earlier
> versions. Soak tests look good.
>
> One last thing I would like to do before we cut 2.12.0:
>
> We are going to be making incompatible changes in 3.0 and I don't think we
> need to telegraph all of the API changes via deprecations in 2.x - most
> notably the recent method name changes of the form s/Time/Duration. I
> understand the rationale for these changes, but they make the 2.x code very
> messy with double deprecations - first from the "millis" methods and then
> from "Time" to "Duration." I think it would be better to keep the existing
> deprecations for the "millis" methods, but drop the new "Duration" ones and
> remove deprecations for the ones they replace. I can see the argument that
> it is better to tell users now, but that takes away flexibility in 3.0 and
> makes the API look very confusing with so many methods that do the same
> thing. Any objections ?
>
> Phil
>
> On Sat, Jul 29, 2023 at 3:59 PM Phil Steitz <ph...@gmail.com> wrote:
>
> > I have run my first round of soak and performance tests on what is now in
> > the 2.x branch. Good news is the code looks stable. Not so good news is
> > it appears that GKOP performance has taken a material hit vs 2.11 and
> > earlier versions. I need to confirm this via more targeted tests and if it
> > turns out not to be real, figure out what is causing it. Hopefully I will
> > get to this done in the next few days.
> >
> > Phil
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org