commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Liam Coughlin <>
Subject Re: [dbutils] Questions about a few of bugs
Date Tue, 10 Feb 2009 22:30:23 GMT

> Ah, but the whole point of this enhancement is that it's unperformant to
> use RowProcessor#toBean on each row; it's much faster to call
> RowProcessor#toBeanList once.  So there's no such thing as an
> OptimizedBeanRowHandler; the optimization is to not use a RowHandler!

I'd push back in the complete opposite direction.  You should _always_ use a
rowhandler and the extra fuctions on row handler should all be collapsed to
a single handleRow function -- ditch toBean and toList etc. all together.

ListHandler<MyBeanType> = new ArrayListHandler<MyBeanType>( new
BeanRowHandler<MyBeanType>() )

and the new theoretical BeanRowHandler would only create the index/property
map once per handled resultset ( the real source of the performance issue in
the first place ).

it's a little verbose on the generics but the guts of processing stream
would be significantly simpler ( and thus -- faster ).

Addidtionally you could have a dbutils extra package wich could have
something like a CglibBeanRowHandler or what have you that could generate
and cache classes to execute resulset->bean mappings.

>  3) CaseInsensitiveMap
>>> Two distinct users claim to want CIM to return the keys in their ORIGINAL
>>> case (i.e. not lowercased).  Why would anyone need this?  I don't get it.
>>>  Given the amount of reflection and property referencing that occurs in
>> java
>> apps these days, i can think of a million reasons why a CIM should return
>> the keys in original case.  There shouldn't be any particular performance
>> implication here, just a little bit of extra memory and housekeeping.
> Well, here's my thinking:
> 1) We don't provide a public CaseInsensitiveMap in our API.  The CIM is a
> purely private object used in the guts of BasicRowProcessor.
> This is why I don't understand the point of the bug...  Why would anyone
> care how this internal object is implemented, so long as BasicRowProcessor
> works?

Well the problem is that BasicRowProcessor.toMap returns a CIM.  -- so if
you use the ListMapHandler or any of the other map result functions you get
a CIM back.  This is useful because developers can then interact with the
result map without worring about column casing from the database -- except
where they care about column casing with the database.... sense?
.entrySet() and .keySet() should of returned Strings in original case in the
first place -- shrug.

> 2) Extra housekeeping = slower performance

True, balancing act.

> 3) The biggest risk here is accidentally introducing thread-safety
> problems.  The patch applied to DBUTILS-34 is not thread-safe (I think?)
> because it keeps two internal data structures (the "real" map and the
> "lowerCaseMap") that can fall out-of-sync, corrupting the map.

I haven't looked at the patch itself, but in the laziest case, the
granularity where the race condition could exist -- the race condition still
exists by itself since CIM extends HashMap rather then hashtable  and so
isn't threadsafe per se, and CIM is never wrapped in a failfast wrapper and
( obviously ) doesn't extent ConcurrentHashMap.

Again -- balancing act -- simple API vs eeking out a couple of small points
of performance.

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message