On 8/29/19 9:47 PM, Romain Manni-Bucau wrote:
Hi

Think both points are valid actually.

Concretely, such a change has perf impacts, indeed compared to a db init it
will be very fast but compared to a driver operation it can be something.
Therefore a microbench comparison does not hurt and will at least guarantee
it is ok and make people calm about it.
When well written lamba are just a static method call so no issue but when
not it allocates way more objects and will stress the gc more which is not
desired for a pool IMHO.

Just my 2cts indeed

What would be great is benchmarks.  I respectfully disagree with the position that pool performance does not matter because other sources of latency swamp it.  For heavily loaded apps, infrastructure performance matters.  We spent a lot of time making DBCP 2 a high-performance pool and it would be a shame to take backward steps now.  What you say above, Roman, indicates that if we do it right we can have it both ways.  That will be great.  We should just confirm performance characteristics.

Phil

Le ven. 30 août 2019 à 01:02, Gary Gregory <garydgreg...@gmail.com> a
écrit :

On Thu, Aug 29, 2019 at 6:17 PM Phil Steitz <phil.ste...@gmail.com> wrote:


On 8/29/19 6:37 AM, Gary Gregory wrote:
Hi All:

In https://github.com/apache/commons-dbcp/pull/34, I've reduced a ton
of
boilerplate code using lambdas. This also happens to fix a bunch of
places
where we did not call checkOpen() when we should have.
Interesting.  I have a couple of comments.

First is a nit - it would make review much easier if you separate the
formatting changes from the actual code changes in commits. Please try
to do that.

Second is more important.  I am not a jvm expert so this may be a
non-issue, but I wonder whether these changes will have performance /
resource utilization impact.  If someone can confirm that this all gets
optimized to the same or equivalent bytecode, there is nothing to worry
about.  If not, I would strongly recommend running benchmarks to
validate that there is no performance impact.

Hi Phil,

I understand your concerns but if we step back to the 10k ft level for this
component, let's remember that we are managing _database_ connections here,
and these database operations are typically extremely slow _compared_ to a
Java application: a database app ends up suffering from dealing delays to
do with network IO plus whatever the database is doing. On top of this, any
DBA would not ever consider allowing an app to use 10k connections, even if
the database could handle it. Maybe measurements would be different with an
in-memory database like H2 or HSQLDB, but the database itself is the
bottleneck no matter what, and borrow/return from DBCP is just noise in
comparison IMO.

My last point is that if I were to write such a component like DBCP
_today_, I would absolutely do it with lambdas, otherwise the duplication
would become a code smell by ignoring a language feature, released in Java
8 and which should be mature on the eve of Java 13.

WRT the nit, yeah, that's unfortunate but some of the files have had _so
much_ boilerplate methods edited into "one-liners" and this is going to
mean a lot of changes indeed. Granted, there might be cases where I only
wanted to reformat one method and mistakenly reformatted a whole file, and
for that I offer my apologies.

Gary


Phil


Gary


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to