Re: Performance improvement proposal. Removal of toLowerCase calls. PR

Jeremy,

At this point send a PR and we can test it. If you are proposing having an optimization switch which defaults to off I can't see why I would not merge it.

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca

On Fri, Jan 17, 2014 at 11:48 AM, Jeremy Whiting <jwhiting@redhat.com> wrote:

Hi Dave and danap,

On 16/01/14 19:33, Dave Cramer wrote:

On Thu, Jan 16, 2014 at 2:21 PM, dmp <danap@ttc-cmc.net> wrote:

Jeremy Whiting wrote:

Hi,
  I would like to propose an optimization to improve performance in the
jdbc driver. The performance improvement has been tested on commodity
hardware using an industry standard Java benchmark. The overall
benchmark metric reports an improvement in performance. Profiling using
sampling showed calls reduced from 1100 to 0 when the benchmark workload
is running.

  The optimization will eliminate calls to the method
toLowerCase(java.util.Locale). This in the pg-jdbc
org.postgresql.jdbc2.AbstractJdbc2ResultSet.findColumnIndex(java.lang.String)
method when setting up the column index registry.

  For the optimization to be enabled I suggest relying on a new system
property. Making the existing functionality the default behaviour to
ensure existing applications do not break when the driver is upgraded.

  The change removes the call toLowerCase when putting items in the
registry [1].  Essentially what's being proposed is removing sanitizing
the key names. For best performance application code should pass SQL to
the driver with column names already folded to lower case.

 I have found this to be incorrect. The column registry is built using fields/columns that have come back from the db.

But upper
case names will still be matched in the second lookup [2] in the method.

****************

  For this optimization to work this feature introduces a requirement on
applications. To use all lower or upper case column names.

****************

This is going to break existing applications, if the requirement is to have
either upper or lowercase.

 Yes it's incompatible. Mixed case situations should use the existing behaviour in 9.3-1100 which is proposed to be the default.

I test explicitly with the MyJSQLView application
the use of mixed case column, key index names because there are applications
that used mixed case. The proper way to handle is quote to handle the mixed
case so it is store as such.

Perhaps the optimization could check for quoting then do no addition process
and store directly intact. Other wise the optimization could initiate as
proposed.

danap.

Are you sure ? This is in the resultset, so any column names should have come back from the db. 

Which means that they should come back in lower case anyway.

I would think the current code would break apps that having real mixed case columns in the database ?

 Column names coming back from the database are sanitized. Before putting into the column registry.

Jeremy's proposal would leave the case alone and store it in the map with mixed case.

 The proposal has a hard requirement of either all upper or lower case columns defined in the database.

 If a user attempts to enable the optimization with an application that uses mixed case the driver does throw an exception of type org.postgresql.util.PSQLException. With this message "The column name <column name> was not found in this ResultSet.".

Jeremy

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca

[2]
https://github.com/pgjdbc/pgjdbc/blob/REL9_3_STABLE/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java?#L2752

--
Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-jdbc

-- 
Jeremy Whiting
Senior Software Engineer, Performance Team
Red Hat

------------------------------------------------------------
Registered Address: Red Hat UK Ltd, 64 Baker Street, 4th Floor, London. W1U 7DF. United Kingdom.
Registered in England and Wales under Company Registration No. 03798903. Directors: Michael Cunningham (USA), Charlie Peters (USA), Matt Parson (USA), Paul Hickey (Ireland)