17

Recently we had a discussion at work about the impact of local variables on the performance vs readability of Java code. Some of my colleagues are of the oppinion that declarations like this

new DoSomethingCmd(new SelectionContext(context, keys), infoStuff.getCurrentRole().getRole_id()).execute(getResultContainer());

will give the application a considerable performance boost. They are willing to sacrifice code readability for this. Are they right in claiming this? Is the above version significantly more performant than, say, this one?

final SelectionContext selectionContext = new SelectionContext(context, keys);
final String roleId = infoStuff.getCurrentRole().getRole_id();
final DeleteSomethingCmd deleteSomethingCmd = new DeleteSomethingCmd(selectionContext,roleId);
deleteSomethingCmd.execute(getResultContainer());

I realise that the first statement isn't all that difficult to grasp in and on itself, but the complexity adds up rather quickly when most of your code is structured like that.

Thank you for your input.

dribnif
  • 175
  • 1
  • 7
  • 3
    If I understood the code at the top right, it appears that your colleagues believe that creating reference variables is slower than creating anonymous ones. Is that right? Why do they think that? – CPerkins Oct 31 '13 at 14:37
  • 7
    I'm happy to leave those sort of optimizations to the built in optimizers. They're good at that. Much better that the source code we look at be easily intelligible. – hatchet - done with SOverflow Oct 31 '13 at 14:40
  • 3
    Unless you have measured otherwise, you should code for clarity and simplicity. This is usually fast enough as well. When you have measured a performance issue only when consider alternative. Often performance optimisations don't help and just add complexity. – Peter Lawrey Oct 31 '13 at 15:00

6 Answers6

15

The only thing that the "optimized" version makes is that you have a few variables less in the stack, slightly increasing your memory consumption. Performance should be measured carefully (google how to benchmark an issue), but I seriously doubt that it has any noticeable effect.

Also, spending time improving performance in a piece of code that is not used often is just a waste of developer time, which is expensive.

In this case, readability should win the day.

EDIT: Anyway, if you use proper indentation, I do not thingk the two versions are too different in readability terms:

new DoSomethingCmd(
    new SelectionContext(context, keys),
    infoStuff.getCurrentRole().getRole_id()
    ).execute(getResultContainer());

The advantage of this text is that you do not have defined variables (selectionContext, roleId)that are no longer needed (so when you read the method again they do not mix with more "persistent" variables). Anyway, that is open to interpretation; the bottom line is that you should not worry with optimization unless you have a motive to do so.

Apart from that, there are some guidelines to Java programming that give you really useful tricks that really help you (v.g. using StringBuilder to concatenate strings).

SJuan76
  • 24,532
  • 6
  • 47
  • 87
  • 6
    Since stack space is pre-allocated, I wouldn’t sign that some variables more on the stack increase memory consumption. You would need thousands of additional variables before an additional block of memory is needed. – Holger Oct 31 '13 at 16:56
  • 1
    An issue with my answer is that it assumes a "dumb" compiler that will give a fixed, different position to each variable. An optimized compiler could find that `roleId` is no longer used and reuse its memory position for, say, `deleteSomethingCmd`, thus reducing even that small benefit. Again, optimization should be checked with a benchmark to verify the actual improvements. – SJuan76 Oct 31 '13 at 18:09
  • 1
    Well, `javac` might reserve slots in a stack frame for them but that are just a few bytes inside a stack that is pre-allocated by the *JVM* with a size of several hundred kB. http://www.oracle.com/technetwork/java/hotspotfaq-138619.html says “As of Java SE 6, this value is 320k in the 32-bit VM and 1024k in the 64-bit VM.” and “64k is the least amount of stack space allowed per thread.” So it doesn’t matter whether we *use* a few bytes more or less, they are *allocated* anyway. – Holger Oct 31 '13 at 18:20
14

Are they right in claiming this?

No, they are not. The cost of storing something in a local variable and then subsequently reading from it is next to 0, it's as simple as that -- this is definitely not something that you should spend your time optimizing, or even worrying about for that matter. Don't sacrifice readability by putting everything in a single, highly convoluted line.

arshajii
  • 127,459
  • 24
  • 238
  • 287
11

A person's time is millions of times more expensive than a computer's time.

If the line of code is expected to execute millions of times more than it will be looked at by a programmer then perhaps you should consider optimising. But certainly not until then.

Even then, it is completely pointless to optimise as if the compiler is stupid.

Don't forget! Premature optimization is the root of all evil.

OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213
4

I see several function calls in here

  • two calls to new

  • a SelectionContext constructor

  • a call to getRole_id()

  • a DeleteSomethingCmd constructor

  • a call to GetResultContainer()

  • a call to the execute() member of the deleteSomethingCmd

So suppose you run this a gazillion times, and in that time you take some stack samples.

What do you think is the probability that a stack sample does not show you inside one of those function calls?

Extremely low, right? Probably far less than one percent?

So even if you made the code at this level infinitely fast, what would it save you?

Perspective is everything.

Mike Dunlavey
  • 40,059
  • 14
  • 91
  • 135
3

The first version is mot much more performant than the second. I doubt you can reliably measure the performance difference. Moreover, after jit optimizations, the result machine code should be the same.

Alexei Kaigorodov
  • 13,189
  • 1
  • 21
  • 38
1

Strictly speaking to the examples, no, there is no measurable difference (not on a modern system, anyway) between the two. However, it is very true that you can get significant performance increases by writing a lot of lower level code to do what can be done with less code at a higher level. For instance, working with arrays is faster in many cases than using any of the Collections API (maybe not much, but it is). But you lose not only readability, but maintainability as well by doing so and usually for an insignificant increase in performance.

Code should always be well organized and readable. Only after getting good performance metrics should you go and look at giving that up for the performance increase.

MadConan
  • 3,749
  • 1
  • 16
  • 27