Re: PgJDBC: code reformat - Mailing list pgsql-jdbc

JMH is an awesome harness for constructing exactly these sorts of performance tests:
http://openjdk.java.net/projects/code-tools/jmh/

If we care about driver performance, it might be interesting to construct
some representative tests and include them as part of the build process, to
ensure that metrics we care about don't get regressions.  Obviously this would be
some work that needs volunteers interested in working on it, just an idea to think on.


And while we're bikeshedding, I personally would -1 the "only one exit per function".

Sometimes it's nice to bail early e.g. :

if (offset == length) { return 0; }
... do work ...
return result;

> On Dec 28, 2015, at 2:02 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>
>
> On 28 December 2015 at 16:16, Gavin Flower <GavinFlower@archidevsys.co.nz> wrote:
> On 29/12/15 01:22, Dave Cramer wrote:
>
>     On 28/12/15 04:01, Vladimir Sitnikov wrote:
>
> [...]
>
>
>         Compare this one:
>         https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java#L2462-L2512
>         with this one:
>         https://github.com/pgjdbc/pgjdbc/blob/197175039068446a15c30d2b5e949f1eae08515d/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java#L264-L311
>
>         Do you see how QueryExecutorImpl.java#L264-L311 is much readable?
>
>     No.  Possibly I'm not seeing the code you want me too?
>
> Same problem here, one of them shows comments. I believe the first one.
>
> Comments can be good, but are orthogonal to the question of code style.
>
>
>     I would add more blank lines for readability, but would eliminate
>     blank lines after opening brackets and before closing brackets.
>
>     Also would simplify lines to ease readability and improve ability
>     to debug, eg
>
>
> So there are very good reasons in Java not to do what you are proposing below. Mostly because it creates objects which have to be garbage collected.
>
> Hmm...
>
> I suspected that the Just-in-Time compiler (I knew from my previous research that they are truly vicious when it comes to optimisation!!!) would end with up with the same code, and actually ran a test that proved it.
>
> I ran multiple loops with and without an intermediate variable, the variation between runs of the same type was vastly greater than the difference in the means - each run was about 34 seconds.
>
> I looked at the output of -XX:CompileCommand=print and found that the optimised code was identical, even though the initial byte code was different!
>
> In essence, adding an intermediate variable makes no difference in the long run.
>
> So making code more readable does NOT lead to any noticeable reduction in performance when the coded is executed many times.  I strongly suspect that the difference in code executed just once in a run, would be insignificant compared to the noise in measuring the run times in practice.
>
> Therefore, since programmer time is costly, it make more sense to put effort into code readability, than into premature micro optimisations.  Which is basically what most texts books on optimisation say.
>
> If you want, I can send you the gory details in an OpenDocument file (e.g. one produced natively by LibreOffice).
>
> While I agree that it is more readable. I think driver code has a responsibility to be as curt as possible with respect to creating objects.
>
> In practice, the changes I have suggested do not have that problem.
>
>
> I stand corrected. Thanks!
>
>
>     Would rewrite:
>         ((V3ParameterList) parameterList).checkAllParametersSet();
>     as
>     V3ParameterList v3ParameterList =(V3ParameterList) parameterList;
>     v3ParameterList.checkAllParametersSet();
>
>
>     and would rewrite:
>         if ((flags &QueryExecutor.QUERY_SUPPRESS_BEGIN)
>     !=0||protoConnection.getTransactionState()
>     !=ProtocolConnection.TRANSACTION_IDLE)
>     as
>         boolean abc =(flags &QueryExecutor.QUERY_SUPPRESS_BEGIN) !=0;
>         boolean xyz = protoConnection.getTransactionState()
>     !=ProtocolConnection.TRANSACTION_IDLE;
>         if (abc||xyz)
>     (except I would use more meaningful names for 'abc' & 'xyz' above!)
>
>     I also only ever have one return per method, as that makes it
>     easier to:
>         (1) understand
>         (2) maintain (add new code and/or change existing code)
>         (3) debug
>
>
>         PgResultSet#getFastLong is very hard to follow no matter which
>         way you
>         format the braces.
>         I believe, "readability" comes from proper segmentation (code
>         blocks
>         vs methods) and proper variable naming.
>
>
>
>
> Dave Cramer
>
> davec@postgresintl.com <mailto:davec@postgresintl.com>
> www.postgresintl.com <http://www.postgresintl.com/>