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

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