You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Gary Gregory (JIRA)" <ji...@apache.org> on 2010/10/21 00:14:25 UTC
[jira] Created: (POOL-173) Better config without duplication.
Better config without duplication.
----------------------------------
Key: POOL-173
URL: https://issues.apache.org/jira/browse/POOL-173
Project: Commons Pool
Issue Type: Improvement
Reporter: Gary Gregory
Priority: Minor
Fix For: 2.0
There is code duplication in configuration code.
I'd like to track an experiment here.
> -----Original Message-----
> From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> Sent: Wednesday, October 20, 2010 10:44
> To: Commons Developers List
> Subject: RE: [pool] Reusing Config
>
> In the same department, I see the following ivars:
>
> lifo : boolean
> maxActive : int
> maxIdle : int
> maxTotal : int
> maxWait : long
> minEvictableIdleTimeMillis : long
> minIdle : int
> numTestsPerEvictionRun : int
> testOnBorrow : boolean
> testOnReturn : boolean
> testWhileIdle : boolean
> timeBetweenEvictionRunsMillis : long
> whenExhaustedAction : WhenExhaustedAction
>
> defined in four classes:
>
> GenericKeyedObjectPool
> GenericKeyedObjectPoolFactory
> GenericObjectPool
> GenericObjectPoolFactory
>
> Which feels to me like a missed opportunity to avoid duplication.
>
> Is making one ivar private or final or volatile be applied to all four
> classes?
>
> We could:
>
> Use a config object instead of the 13 ivars.
> Or a common superclass then we can consider if it should hold the ivar list or
> a Config object.
>
> Would it be too weird to have a common super class for BaseObjectPool and
> BasePoolableObjectFactory for example?
>
> Gary Gregory
> Senior Software Engineer
> Rocket Software
> 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> Tel: +1.404.760.1560
> Email: ggregory@seagullsoftware.com
> Web: seagull.rocketsoftware.com
>
>
>
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:29
> > To: Commons Developers List
> > Subject: [pool] Reusing Config
> >
> > Hi All:
> >
> > I think this came up recently. Any thoughts or plans on extracting the
> Config
> > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> reused.
> > The constants for default values could then also be moved to Config.
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> >
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gary Gregory updated POOL-173:
------------------------------
Attachment: pool2config.diff
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Steve Siebert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12923740#action_12923740 ]
Steve Siebert commented on POOL-173:
------------------------------------
Hmmm...sorry about that last "out of nowhere" comment...I didn't think adding a dependency link from the previous ticket would generate a "thread" comment =).
Anyway, I just uploaded the very basic class diagram for how things currently are - I think it easily points out some of the significant issues Gary pointed out. It also exposes some other issues that may need to be investigated. Unfortunately, the constructor API is already set for the 2.0 release...depending on how we go with the config, we could probably drop a lot of these constructors. We might consider using a Builder pattern to ease the programmatic creation of the configuration.
I'm looking at the diff right now...I'm getting only a partial apply, two errors:
C:\development\commons-pool\trunk\src\java\org\apache\commons\pool2\impl\GenericKeyedObjectPoolFactory.java (Cannot apply hunk @@ 434 )
C:\development\commons-pool\trunk\src\java\org\apache\commons\pool2\impl\GenericObjectPoolFactory.java (Cannot apply hunk @@ 518 )
I'll go over the patch and provide some feedback =).
As I said in the email thread, there are a lot of other issues with configuration (initial provisioning/properties file) that we might want to consider while doing this re-look. I'll provide some comments on this as well, but if I start straying or trying to incorporate too much too fast, let me know =)
S
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gary Gregory updated POOL-173:
------------------------------
Attachment: pool2config.diff
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Steve Siebert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12929535#action_12929535 ]
Steve Siebert commented on POOL-173:
------------------------------------
HI Phil,
I am trying to finish up an alternate implementation, where the config instance stays as a reference, to see which approach will be better performant under load (I think the key will be the way each does synchronization). I'm spinning some cycles on the builders with inheritance, trying to make it as clean as possible. I hope to have the patch completed soon and I'll do the tests and provide the results =)
Thanks!
Steve
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, genericObjectPoolConfig.patch, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gary Gregory updated POOL-173:
------------------------------
Attachment: (was: pool2config.diff)
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Sebb (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12923390#action_12923390 ]
Sebb commented on POOL-173:
---------------------------
There are a lot of unrelated changes which make it very hard to follow the diff file.
For example, comments have been rewrapped, and local variables renamed; using this.variable instead of plain variable.
Would it be possible to show necessary changes only?
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gary Gregory updated POOL-173:
------------------------------
Attachment: pool2config.diff
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Simone Tripodi updated POOL-173:
--------------------------------
Attachment: pool173-better_config_without_duplication.diff
I invested my spare time to rearrange the concepts we've been discussing on ML; please have a look at the attached patch {{pool173-better_config_without_duplication.diff}}, unfortunately I didn't find a good (and free) UML plugin for Eclipse for creating a diagram to show you the benefits introduced by adding and sharing the configuration objects.
I didn't add the JMX interfaces since are object of another issue, but at the new stage they should be added easily.
Javadoc is updated and tests have been updated and are running, please let me know if it could be the foundation of the refactoring so I can commit the code.
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (POOL-173) Better config without duplication.
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz reopened POOL-173:
------------------------------
Assignee: (was: Simone Tripodi)
Here is a patch showing how we can use the Config encapsulation while maintaining granular control of pool properties. The patch is against trunk and only shows changes to GOP. If the approach is acceptable, we can extend to GKOP. I did not rename properties or reduce mutability per discussions on commons-dev. That would be a logical next step.
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gary Gregory updated POOL-173:
------------------------------
Attachment: (was: pool2config.diff)
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Steve Siebert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Steve Siebert updated POOL-173:
-------------------------------
Attachment: commons-pool2-configExisting.dia
commons-pool2-configExisting.png
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Steve Siebert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12924540#action_12924540 ]
Steve Siebert commented on POOL-173:
------------------------------------
I took a look at this last night but didn't get a chance to comment =)
I like the patch, I believe this does indeed satisfy the issue.
One question I have, since we're eliminating the primitive configuration properties within the pool/factory classes, we're making the Config objects publicly accessible, and possibly accessing through JMX is the idea of making the Config objects thread-safe. This would certainly reduce the need to have to externally synchronize (and possibly introduce bugs) every time.
Another issue we probably need to open another ticket for is to deprecate the constructors we've eliminated in 1.5.
Last suggestion/question is about making inner (public static final) Builder pattern classes within the concrete Config classes (and possibly defining an abstract <T extends Abstract*Config> create() method in the Abstract class). This would further simplify the programmatic creation of the Config classes.
Thoughts?
+1 on Simones patch...we can add any of the above after it has been committed.
S
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12923482#action_12923482 ]
Gary Gregory commented on POOL-173:
-----------------------------------
It is possible but I am not going to take the time to do this right now. I would have to start over and be careful not to step on this and that toe. It is just too big of a wholesale change set.
My goal here is to use this as a workspace to share my experiment. I do not consider it 100% complete but it is mostly there. For example the next thing to try with this patch is: see what it feels like to replace ivars in GenericObjectPool GenericKeyedObjectPool with a GenericObjectPoolConfig object. As Simone says, the two config interfaces could be merged into one too.
I was thinking other folks would just compare the end result class hierarchies and see how they smelled and felt. Without getting caught up in micro changes.
I am offering this patch as an illustration of what direction I see this going based on what I started in the email thread.
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12924530#action_12924530 ]
Phil Steitz commented on POOL-173:
----------------------------------
Great work, guys!
I agree with Gary that we don't need all of the primitive type-based constructors and even with Simone's elimination of them altogether. Can anyone see a reason that we need to retain these?
+1 for committing Simone's patch as a start. As I said on the ML, we may want to look at redefining some of the config parameters, but for now, this refactoring of the constructors and config setup is a good start, IMO.
I have never liked the stateful factories, which look even more silly now once constructors just use Config objects. I am tempted to suggest that we make them stateless; but I know that will cause some trauma in the DBCP JNDI setup. I have not fully thought through how to work around that problem, but I am sure there is a way.
For now, unless others see problems with it, I suggest Simone commits the last patch above, we can resolve this issue and open others for further improvements.
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (POOL-173) Better config without duplication.
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12923794#action_12923794 ]
Gary Gregory commented on POOL-173:
-----------------------------------
Thank you for the diagrams. I do not like the constructor zoo either.
IMO, these constructors should allow to build legal instances. Since there are default values for all config options, I would argue that we need one constructor that lets you set them all and one that lets you set none which means you get the defaults. I would also have a constructor that accepts a config object.
For example, instead of:
- GenericObjectPool(PoolableObjectFactory<T>)
- GenericObjectPool(PoolableObjectFactory<T>, Config)
- GenericObjectPool(PoolableObjectFactory<T>, int)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, boolean, boolean)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int, boolean, boolean)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int, boolean, boolean, long, int, long, boolean)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int, int, boolean, boolean, long, int, long, boolean)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int, int, boolean, boolean, long, int, long, boolean, long)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int, int, boolean, boolean, long, int, long, boolean, long, boolean)
You only need:
- GenericObjectPool(PoolableObjectFactory<T>)
- GenericObjectPool(PoolableObjectFactory<T>, Config)
- GenericObjectPool(PoolableObjectFactory<T>, int, WhenExhaustedAction, long, int, int, boolean, boolean, long, int, long, boolean, long, boolean)
The other constructors are just noise IMO.
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (POOL-173) Better config without duplication.
Posted by "Simone Tripodi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Simone Tripodi resolved POOL-173.
---------------------------------
Resolution: Fixed
Patch applied, please see revision 1026616.
I'm now opening 2 new issues for thread-safety support on Config classes and adding inner Builders
Thanks everybody for the feedbacks!
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Assignee: Simone Tripodi
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (POOL-173) Better config without duplication.
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/POOL-173?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz updated POOL-173:
-----------------------------
Attachment: genericObjectPoolConfig.patch
> Better config without duplication.
> ----------------------------------
>
> Key: POOL-173
> URL: https://issues.apache.org/jira/browse/POOL-173
> Project: Commons Pool
> Issue Type: Improvement
> Reporter: Gary Gregory
> Priority: Minor
> Fix For: 2.0
>
> Attachments: commons-pool2-configExisting.dia, commons-pool2-configExisting.png, genericObjectPoolConfig.patch, pool173-better_config_without_duplication.diff, pool2config.diff
>
>
> There is code duplication in configuration code.
> I'd like to track an experiment here.
> > -----Original Message-----
> > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > Sent: Wednesday, October 20, 2010 10:44
> > To: Commons Developers List
> > Subject: RE: [pool] Reusing Config
> >
> > In the same department, I see the following ivars:
> >
> > lifo : boolean
> > maxActive : int
> > maxIdle : int
> > maxTotal : int
> > maxWait : long
> > minEvictableIdleTimeMillis : long
> > minIdle : int
> > numTestsPerEvictionRun : int
> > testOnBorrow : boolean
> > testOnReturn : boolean
> > testWhileIdle : boolean
> > timeBetweenEvictionRunsMillis : long
> > whenExhaustedAction : WhenExhaustedAction
> >
> > defined in four classes:
> >
> > GenericKeyedObjectPool
> > GenericKeyedObjectPoolFactory
> > GenericObjectPool
> > GenericObjectPoolFactory
> >
> > Which feels to me like a missed opportunity to avoid duplication.
> >
> > Is making one ivar private or final or volatile be applied to all four
> > classes?
> >
> > We could:
> >
> > Use a config object instead of the 13 ivars.
> > Or a common superclass then we can consider if it should hold the ivar list or
> > a Config object.
> >
> > Would it be too weird to have a common super class for BaseObjectPool and
> > BasePoolableObjectFactory for example?
> >
> > Gary Gregory
> > Senior Software Engineer
> > Rocket Software
> > 3340 Peachtree Road, Suite 820 . Atlanta, GA 30326 . USA
> > Tel: +1.404.760.1560
> > Email: ggregory@seagullsoftware.com
> > Web: seagull.rocketsoftware.com
> >
> >
> >
> > > -----Original Message-----
> > > From: Gary Gregory [mailto:GGregory@seagullsoftware.com]
> > > Sent: Wednesday, October 20, 2010 10:29
> > > To: Commons Developers List
> > > Subject: [pool] Reusing Config
> > >
> > > Hi All:
> > >
> > > I think this came up recently. Any thoughts or plans on extracting the
> > Config
> > > class out of GenericKeyedObjectPool and GenericObjectPool so it can be
> > reused.
> > > The constants for default values could then also be moved to Config.
> > > Gary Gregory
> > > Senior Software Engineer
> > > Rocket Software
> > > 3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
> > > Tel: +1.404.760.1560
> > > Email: ggregory@seagullsoftware.com<ma...@seagullsoftware.com>
> > > Web: seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/>
> > >
> >
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.