3

I have a class

class A {
    private int x;
    public void setX(...){...}
    public int getX(){return x;}
}

class B {
    int y;
    public void setY() {
        //Accessing x of A, assume I already have object of A
        if(a.getX() < 0) {
             y = a.getX();
        }
    }
}

class C {
    int y;
    public void setY() {
        //Accessing x of A, assume I already have object of A
        int tmpX = a.getX();
        if(tmpX < 0) {
             y = tmpX;
        }
    }
}

Which one is better way of coding? The way I have accessed x of A in class B or in class C?

Val
  • 1
  • 8
  • 40
  • 64
Chandra Sekhar
  • 18,914
  • 16
  • 84
  • 125
  • The compiler is smart, trust him. It'll convert them to the same byte-code. – Maroun Jun 28 '13 at 11:49
  • Both seem almost equal to me. – Ved Jun 28 '13 at 11:49
  • class `B` and `C` don't extends `A`, they dont have access – Shaddow Jun 28 '13 at 11:49
  • Probably I would go for `C` and also declare `int tmpX` as final if we don't modify it anywhere else in the code. – AllTooSir Jun 28 '13 at 11:50
  • 1
    Before you think about micro optimizing you should take a look at "What does the JIT (Just in time) compiler do" http://stackoverflow.com/questions/95635/what-does-a-just-in-time-jit-compiler-do . In other words: as long as you don't have real performance issues you should prefer a clear design over code optimization. – René Link Jun 28 '13 at 11:50
  • 1
    This question is already asked and answered [here](http://programmers.stackexchange.com/questions/201398/if-i-have-many-calls-of-single-method-that-returns-field-value-is-it-better-to/201401#201401). – Uwe Plonus Jun 28 '13 at 11:51
  • The compiler might actually see that there are two reads done in the first approach and optimize it to the second approach you have written. In the end it would result the same bytecode. – gaganbm Jun 28 '13 at 11:51
  • Both of them are written in same purpose. B and C is the same – Ugur Artun Jun 28 '13 at 11:52
  • 3
    @gaganbm - It would be illegal for the compiler to generate the first case the same as the second. Where a method call appears in the source, it must be evaluated. There is, after all, no guaranteed that getX returns the same value each time, nor is there a guarantee that getX does not modify some internal value in a. – Hot Licks Jun 28 '13 at 11:55
  • @HotLicks Oh, yes. You are right. – gaganbm Jun 28 '13 at 11:57

11 Answers11

12

Let's look at what it compiles to. I compile

class A {
    private int x;
    public void setX(int x_){x=x_;}
    public int getX(){return x;}
}

class B {
    int y;
    A a;
    public void setY() {
        //Accessing x of A, assume I already have object of A
        if(a.getX() < 0) {
             y = a.getX();
        }
    }
}

class C {
    int y;
    A a;
    public void setY() {
        //Accessing x of A, assume I already have object of A
        int tmpX = a.getX();
        if(tmpX < 0) {
             y = tmpX;
        }
    }
}

And get for B

  public void setY();
    Code:
       0: aload_0       
       1: getfield      #2                  // Field a:LA;
       4: invokevirtual #3                  // Method A.getX:()I
       7: ifge          21
      10: aload_0       
      11: aload_0       
      12: getfield      #2                  // Field a:LA;
      15: invokevirtual #3                  // Method A.getX:()I
      18: putfield      #4                  // Field y:I
      21: return        
}

and for C

  public void setY();
    Code:
       0: aload_0       
       1: getfield      #2                  // Field a:LA;
       4: invokevirtual #3                  // Method A.getX:()I
       7: istore_1      
       8: iload_1       
       9: ifge          17
      12: aload_0       
      13: iload_1       
      14: putfield      #4                  // Field y:I
      17: return        
}

As C only calls getX once it will be more "efficient" as this is the most expensive thing there. However you really won't notice this. Especially as the HotSpot JVM will "inline" this method call very quickly.

Unless this is the main bit of code being run There's no point optimising this as you will barely notice it.

However, as mentioned elsewhere there are other reasons beyond performance why the C approach is preferable. One obvious one is if the result of getX() changes inbetween the two calls (in the presence of concurrency).

selig
  • 4,834
  • 1
  • 20
  • 37
  • 2
    embedded system does not use hot spot – AlexWien Jun 28 '13 at 11:56
  • Sorry, I assumed it was using standard a JVM (no indication otherwise in question). Not really worth optimising anyway. But I suppose in an embedded system perhaps - `C` is better anyway. – selig Jun 28 '13 at 11:58
  • Sorry - I meant the way class `C` does it. One virtual call versus two. – selig Jun 28 '13 at 12:04
  • Good work doing the bytecode analysis, but I disagree with the conclusion of this answer for two reasons a) the efficiency argument assumes the best possible case (perfect JIT optimisation) and there are many situations where it *will* make a performance difference and b) there are other reasons to prefer use of a temporary variable (concurrency effects, maintainability) – mikera Jun 28 '13 at 12:27
2

Which one is better way of coding?

In terms of readability, it is debatable but there is little difference.

In terms of robustness, C is better; see below (at the end), though you can often rule out those scenarios.

In terms of performance (which is what you are really asking about), the answer is that it is platform dependent. It depends on:

  • whether you are compiling or interpreting the code,
  • if you are JIT compiling whether that code actually gets compiled or not, and
  • the quality of the compiler / optimizer, and its ability to effectively optimize.

The only way to be sure is to create a valid micro-benchmark and actually test the performance using the specific platform that you are concerned about.

(It also depends on whether getX() needs to be a virtual call; i.e. whether is a subclass of X that overrides the getX() method.)

However, I would predict that:

  • on a Java Hotspot system with JIT compilation enabled, the JIT will inline the getX() calls (modulo the virtual call issue),
  • on an early Davlik VM, the JIT compiler won't inline the call, and
  • on a recent Davlik VM, the JIT compiler will inline the call.

(The last prediction is based on this Answer from one of the Davlik compiler guys ... )


It is generally a bad idea to preemptively micro-optimize your code:

  • Most of the time, the micro-optimization will be a waste of time. Unless this code is executed a lot, any performance difference is likely not to be noticeable.
  • Some of the rest of the time, the micro-optimization will be ineffective ... or actually make things worse1.
  • Even if your micro-optimization works on one generation of your platform, JIT compiler changes in later versions may render the micro-optimizations ineffective ... or worse.

1 - I have seen advice from Sun compiler guys to the effect that "clever micro-optimizations" can actually prevent the optimizer from detecting that a useful optimization is possible. This probably doesn't apply in this example, but ...


Finally, I would note that there are circumstances in which B and C are not equivalent code. One circumstance that springs to mind is if someone creates a subclass of A where the getX method has a hidden side-effect; e.g. where calling getX causes an event to be published, or increments a call counter.

Community
  • 1
  • 1
Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
1

C is more efficient because the getters is called once.

User Hot Licks commented that the compiler cannot optimize the second call, because it cannot know whether getX() would deliver another result in the second call.

In your example its not much differeence, however in loops it is.

User selig proved the asumptions, he decompiled and showed that C is more efficient, because B calls the method twice.)

AlexWien
  • 28,470
  • 6
  • 53
  • 83
1

You should normally use the temporary variable, i.e. the following is usually better:

 int tmpX = a.getX();
 if(tmpX < 0) {
       y = tmpX;
 }

There are several reasons for this:

  • It will be at least as fast or faster. Using a temporary local int variable is super-cheap (most likely stored in a CPU register) and better than the cost of an additional method call plus an additional field lookup. If you are lucky then the JIT may compile the two down to equivalent native code, but that is implementation dependent.
  • It is safer for concurrency - the field x may get changed by another thread in between the two getX() calls. Normally you want to read a value just once, and work with that value rather than have the problem of dealing with two potentially different values and confusing results....
  • It will definitely be more efficient if somebody goes and makes the getX() call more complicated in the future (e.g. adding logging, or computing the value of x rather than using a field). Think long-term maintainability.
  • You can use a better name by assigning to a well-named temporary variable. tmpX isn't really very meangingful, but if it was something like playerOneScore then it would make your code much clearer. Good names make your code more readable and maintainable.
  • It is good practice in general to minimise superfluous method calls. Even if it doesn't matter in this particular case, it is better to get into the habit of doing this, so that you do it automatically in situations where it matters (e.g. when the method call causes an expensive database lookup).
mikera
  • 105,238
  • 25
  • 256
  • 415
0

In the title, you're asking which is more efficient. I take it you mean performance-wise. In that case, for a typical getter that simply exposes a field I'd be surprised if the two cases turned out to be any different.

Better way of coding, on the other hand, tends to refer to readability and structuring. In that case, I'd personally go for the second.

Theodoros Chatzigiannakis
  • 28,773
  • 8
  • 68
  • 104
0

The method in class B will call the method two times but the method in class C will call it once.. So Class C approach is better

Pranav Kale
  • 629
  • 1
  • 5
  • 13
0

Up until the assignment to y they're identical -- the temp var has no effect (since one is generated internally in the first case).

However, the first case will cause (by Java rules) another invocation of getX for the assignment to y, whereas the second will reuse the previous value.

(But a JITC may flatten this and make them the same again.)

Note: It's important to understand, though, that the two versions are not semantically identical. They do different things and can have different results.

Hot Licks
  • 47,103
  • 17
  • 93
  • 151
0

if x and y are coordinates which you need very often, consider direct access: if you have a getter and a setter , then you can made them public or protected, too.

 if (a.x < 0) {
    y = a.x;
 }

that might be look a bit anti object oriented, but in moderen languages you have properties to avoid that ugly getters in formulas. The code is much more readable than your duplicate getX().

(a.getX() + b.getX() + c.getX()) / 3.0;

is not so easy to proof if beeing correct than:

(a.x + b.x + c.x) / 3.0;
AlexWien
  • 28,470
  • 6
  • 53
  • 83
0

If you really really care your best bet is to code up a quite test and find out which executes fastest. The problem is that the result could change depending on which version of the VM you are using.

My best guess would be that class c is slightly better than b because it only requires a single method call. If you finalize the temporary int you might even get slightly better performance. I once tested this

for( int i = 0; i < foo.size(); i++ )

against

for( int i = 0, n = foo.size(); i < n; i++ )

and found the latter to be preferable (it was an argument with another programmer, I won). The situation you have is probably very similar as I'm guess you wouldn't be worrying about this unless you are creating millions of class b or c objects. If you aren't creating million of class b / c objects then I'd worry about something else as you aren't going to make any noticeable difference.

wobblycogs
  • 4,083
  • 7
  • 37
  • 48
0

If you want to check for yourself you can use a System.currentTimeMillis() and then run the code a couple of million times (each time first setting any created variable into something else to ensure it is reset) then using System.currentTimeMillis() again and subtracting to get total time repeat for each of them to see which is faster. By the way I doubt it will make a big difference unless you are actually going to run this millions of times.

Thijser
  • 2,625
  • 1
  • 36
  • 71
0

This Answer is solely to address a point raised in this comment:

It would be illegal for the compiler to generate the first case the same as the second. Where a method call appears in the source, it must be evaluated. There is, after all, no guaranteed that getX returns the same value each time, nor is there a guarantee that getX does not modify some internal value in a. – Hot Licks Jun 28 at 11:55

Here's the code at issue:

    if(a.getX() < 0) {
         y = a.getX();
    }

where getX() is

    public int getX(){return x;}

(This method is clearly side-effect free.)

In fact, the compiler is allowed to optimize the 2nd call away, assuming that it can deduce that nothing in the current thread will can alter the result. It is allowed to ignore changes made by another thread ... unless there is the action that made the relevant state change "happens before" the action that observed the state. (In other words, unless the change is made in a thread-safe fashion.)

In this case, the code is clearly not thread-safe. And it therefore follows that the compiler (or more precisely, the JIT compiler) is permitted to optimize away the 2nd call.

However, the bytecode compiler is not permitted to make this optimization. The two classes are separate compilation units, and the bytecode compiler has to allow for the possibility that (say) A could be modified and recompiled after B has been recompiled. Thus, the bytecode compiler cannot be certain that A.getX() is always going to be side-effect free while compiling B. (By contrast, the JIT can make this deduction ... since the classes can't change after they have been loaded.)

Note that this is just about what the compilers are permitted to do. In practice, they are liable to be more conservative, not least because these optimizations tend to be relatively expensive to perform.


I don't know how the JIT compiler's optimizers work, an obvious approach would be like this;

  1. deduce that getX() is a method that doesn't require a virtual method dispatch, and therefore a candidate for inlining
  2. inline the method body into the call at both points
  3. perform a local data flow analysis which shows that the same variable is loaded twice in the space of a few instructions
  4. on the basis of that, eliminate the second load.

So in fact, the second call could be entirely optimized away with explicitly reasoning about the method's possible side-effects.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216