Ariel,

Thanks for all the extra info. It should help me make better decisions with
Java in the future. I will see if I can move the instrument check to a
static boolean and return the rest of the code to it's original form.

My comment was not intended as a cop-out. I was speaking from experience. I
was on a project once where the focus was exclusively on performance. The
developers played all kinds of tricks to make sure the compiler generated
the most efficient code possible including tricks like right-shift instead
of divide. Then, a few years later a different regime took over the product
and focused exclusively on readability arguing that the compiler should and
will address performance issues. Customers determined that the performance
of the product dropped by 20% and they were furious. Now it was not any one
little thing that caused the drop. It was hundreds of little things.
Perhaps a better choice of words would have been 'cultural' rather than
'philosophical'.

All that said, you are right that I should better understand the
optimization strategies of the JVM before making additional optimizations
myself.

Thanks,

-Walter


On Mon, Mar 31, 2014 at 9:40 PM, aweisberg <notifications@github.com> wrote:

> In src/frontend/org/voltcore/network/VoltNetwork.java:
>
> >          try {
> >              while (m_shouldStop == false) {
> >                  try {
> > -                    while (m_shouldStop == false) {
>
> Hi, If you want to know what happens inlining wise use -XX:+PrintInlining.
> but will always have the same outcome so that path will be inlined How is
> the compiler going to figure that out? How do you know one path will be
> eliminated? I am 99% sure it can't because it can't derive that there is
> some instance of the class where the value is null. It's also not going to
> inspect the constructor, inline the the method for generating the set, and
> derive that the result of that function is a constant. If you want to make
> the selector check compile out you would set the result of the reflection
> checks in NinjaSelector.instrumentSelector to a static final boolean.
> Conditionals based on static final booleans are the only reliable way to
> make sure that something is eliminated. There is nothing else that would
> allow elimination because as far as the runtime knows the other branch
> might still get hit because it doesn't know if there is some class instance
> that might still take that branch. As far as it knows you have some other
> random class that is going to reset the value using reflection, seriously,
> there are a lot of frameworks that do this and that is why the JVM has to
> be conservative, there is no such thing as final for member fields.
> philosophical and cultural debate That is a cop out. Optimizations that
> don't fit with how the language and runtime work aren't helpful.
> Duplicating code is not just a maintenance headache and source of bugs, it
> is inefficient as well. It's more code to cache and prevents the JVM from
> inlining the method. The tools available are way to good for us to be
> wandering around in the dark. If estimated time offends you create a task
> for periodic priority work to update the estimated time. That would be a
> nice improvement, there was no periodic threads when the EstTime stuff was
> made. Thanks, Ariel
> ... <#1451af1ea53a8fa7_>
> On Mon, Mar 31, 2014 at 8:43 PM, wweiss-voltdb <notifications@github.com>wrote:
> In src/frontend/org/voltcore/network/VoltNetwork.java: > try { > while
> (m_shouldStop == false) { > try { > - while (m_shouldStop == false) { I am
> not familiar enough with the JIT/JVM to say. In practice the case I am
> trying to compile out (the selector test is the the deepest in the code but
> will always have the same outcome so that path will be inlined and remove
> an unnecessary element from the data cache. The other two tests are harder
> to compile out because both paths will be used. So for that path, I don't
> know if saving a condition is worth the extra code cost. All that said, if
> either optimization is sacrificing readability that is a philosophical and
> cultural debate I have had many times and that I have no opinion on. For me
> it is just a question of understanding that point in the continuum that is
> being observed here. So I am happy to do whatever you think is appropriate.
> Unless you tell me otherwise, I will assume you want to just add a
> conditional for the selector to the original code. Thanks, Walter ...
> <#1451abd7f0a87a9e_> On Mon, Mar 31, 2014 at 6:31 PM, aweisberg <
> notifications@github.com> wrote: > In
> src/frontend/org/voltcore/network/VoltNetwork.java: > > > try { > > while
> (m_shouldStop == false) { > > try { > > - while (m_shouldStop == false) { >
> > Hi, I think what you are doing is an optimization at the cost of clarity
> > and also not an optimization in some dimensions. Copying and pasting code
> > results in more code which prevents the JVM from inlining as
> aggressively. > If this is to save two comparison it doesn't make much
> sense to me. You > could always benchmark with JMH and see how much it
> actually buys you. I > think that pushing stuff into methods is fine, but
> duplicating is not. I > think this makes it harder to introduce new code.
> If I wanted to implement > checks to spin if there was a steady stream of
> work it would be trickier > and I would have to do it multiple times.
> Thanks, Ariel > ... <#1451a4512ff85c64_> > On Mon, Mar 31, 2014 at 6:10 PM,
> wweiss-voltdb <notifications@github.com>wrote: In
> src/frontend/org/voltcore/network/VoltNetwork.java: > try { > while
> (m_shouldStop == false) { > try { > - while (m_shouldStop == false) { Hi
> Ariel, I have changed to code to minimize the loops. The only way to reduce
> code further is with method parameters and I don't know how or if they get
> optimized so if you think it is still to complex, I will just put it back
> the old way and and a conditional in the main loop. Thanks, ...
> <#1451a31c64094e8b_> -Walter On Fri, Mar 28, 2014 at 5:46 PM, aweisberg <
> notifications@github.com> wrote: In
> src/frontend/org/voltcore/network/VoltNetwork.java: > try { > while
> (m_shouldStop == false) { > try { > - while (m_shouldStop == false) {
> Duplicating the loop is way too much copy pasta to remove one if statement.
> Pulling the estimate time stuff into a separate function makes sense
> though. I would push the condition for different key set types into an
> invokeCallbacks wrapper. -- Reply to this email directly or view it on
> GitHub<https://github.com/VoltDB/voltdb/pull/1252/files#r11087128> . --
> *Walter Weiss,* Software Engineer *VoltDB, Inc.* *E:* wweiss@voltdb.com*O:*
> +1 978.528.0592 *M:* +1 978.204.4745 www.voltdb.com< http://voltdb.com>
> Follow VoltDB on Twitter <https://twitter.com/voltdb>, Facebook<
> https://www.facebook.com/VoltDB?sk=wall>, and LinkedIn <
> http://www.linkedin.com/company/voltdb> -- Reply to this email directly
> or view it on GitHub<
> https://github.com/VoltDB/voltdb/pull/1252/files#r11139667> . -- Reply to
> this email directly or view it on GitHub<
> https://github.com/VoltDB/voltdb/pull/1252/files#r11140451> . -- *Walter
> Weiss,* Software Engineer *VoltDB, Inc.* *E:* wweiss@voltdb.com *O:* +1
> 978.528.0592 *M:* +1 978.204.4745 www.voltdb.com< http://voltdb.com>
> Follow VoltDB on Twitter <https://twitter.com/voltdb>, Facebook<
> https://www.facebook.com/VoltDB?sk=wall>, and LinkedIn <
> http://www.linkedin.com/company/voltdb> -- Reply to this email directly
> or view it on GitHub<
> https://github.com/VoltDB/voltdb/pull/1252/files#r11143996> .
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/VoltDB/voltdb/pull/1252/files#r11144976>
> .
>



-- 

*Walter Weiss,* Software Engineer
*VoltDB, Inc.*
*E:* wweiss@voltdb.com
*O:* +1 978.528.0592
*M:* +1 978.204.4745
www.voltdb.com <http://voltdb.com>
Follow VoltDB on Twitter <https://twitter.com/voltdb>,
Facebook<https://www.facebook.com/VoltDB?sk=wall>,
and LinkedIn <http://www.linkedin.com/company/voltdb>