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.
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