On 1-Nov-06, at 8:19 PM, Michael Paesold wrote:
> Hi Dave,
>
> first, I forgot to say that I did do performance tests using the
> previously posted test case. The new version is definitely faster
> than the original code, although only marginally. When doing my
> first version of the patch, I tested different code constructs for
> speed. I chose the one with the best performance.
>
> I ran the tests with both 1.4.2 and 1.5.0. It might be possible
> that the performance will be worse in an earlier versions of the
> JDK with a less sophisticated hot-spot compiler.
Cool my comments were pretty minor anyway.
>
> Dave Cramer wrote:
>> Michael,
>> I'm only looking at this from a performance standpoint.
>> 1) testing for array.length in a loop is fairly expensive compared
>> to testing for 0, not a big deal but it adds up.
>
> Could you give me a hint what part of the code you were looking at?
> And what you would want me to change? Do you think we should scan
> the query backwards (testing the offset against zero -- not sure if
> this is a good idea) or do you want to make the query char[] zero-
> terminated so that we can test against '\0'?
>
> Btw. changing the code to
> int len = array.length;
> for (....; i < len ; ...)
> did not give any performance advantage for me. I think the compiler
> can quite effectively optimize access to .length here.
>
>
>> 2) I'm wondering what the cost of a switch is for two cases
>> ( candidly I don't know the answer to this,and it's quite likely
>> that modern compilers will turn it into an if else anyway.)
>
> Hmm, perhaps it's bad coding style that I left out the empty
> default. But I only see this example in the code (2 cases + an
> implicit default):
>
> + switch (query[offset])
> + {
> + case '\\':
> + ++offset;
> + break;
> + case '\'':
> + return offset;
> + }
>
> That would really translate to:
> if (query[offset] = '\\') ... else if (query[offset] = '\'') ... /*
> else nothing */
>
> Should I add an empty default in that code sections? When I
> originally wrote that code, it was all in that main loop, so I
> wanted to keep it short. Now that it is split into several methods,
> I see no reason for brevity.
>
> Best Regards,
> Michael Paesold
>
>
>
>
>