Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

There is a source file in a rather large project with several functions that are extremely performance-sensitive (called millions of times per second). In fact, the previous maintainer decided to write 12 copies of a function each differing very slightly, in order to save the time that would be spent checking the conditionals in a single function.

Unfortunately, this means the code is a PITA to maintain. I would like to remove all the duplicate code and write just one template. However, the language, Java, does not support templates, and I'm not sure that generics are suitable for this.

My current plan is to write instead a file that generates the 12 copies of the function (a one-use-only template expander, practically). I would of course provide copious explanation for why the file must be generated programmatically.

My concern is that this would lead to future maintainers' confusion, and perhaps introduce nasty bugs if they forget to regenerate the file after modifying it, or (even worse) if they modify instead the programmatically-generated file. Unfortunately, short of rewriting the whole thing in C++, I see no way to fix this.

Do the benefits of this approach outweigh the disadvantages? Should I instead:

  • Take the performance hit and use a single, maintainable function.
  • Add explanations for why the function must be duplicated 12 times, and graciously take the maintenance burden.
  • Attempt to use generics as templates (they probably don't work that way).
  • Yell at the old maintainer for making code so performance-dependent on a single function.
  • Other method to maintain performance and maintainability?

P.S. Due to the poor design of the project, profiling the function is rather tricky... however, the former maintainer has convinced me that the performance hit is unacceptable. I assume by this he means more than 5%, though that is a complete guess on my part.


Perhaps I should elaborate a bit. The 12 copies do a very similar task, but have minute differences. The differences are in various places throughout the function, so unfortunately there are many, many, conditional statements. There are effectively 6 "modes" of operation, and 2 "paradigms" of operation (words made up by myself). To use the function, one specifies the "mode" and "paradigm" of operation. This is never dynamic; each piece of code uses exactly one mode and paradigm. All 12 mode-paradigm pairs are used somewhere in the application. The functions are aptly named func1 to func12, with even numbers representing the second paradigm and odd numbers representing the first paradigm.

I'm aware that this is just about the worst design ever if maintainability is the goal. But it seems to be "fast enough", and this code hasn't needed any changes for a while... It's also worth noting that the original function has not been deleted (although it is dead code as far as I can tell), so refactoring would be simple.

share|improve this question
    
If the performance hit is serious enough to warrant 12 versions of the function, then you might be stuck with it. If you refactor to a single function (or use generics), would the performance hit be bad enough that you'll lose customers and business? –  FrustratedWithFormsDesigner Jun 24 at 18:06
11  
Then I think you need to do your own tests (I know you said profiling is tricky, but you can still do rough "black-box" performance tests of the system as a whole, right?) to see just how big a difference there is. And if it's noticable, then I think you might be stuck with the code generator. –  FrustratedWithFormsDesigner Jun 24 at 18:11
1  
This could sounds like it could have benefited from some parametric polymorphism (or maybe even generics), rather than calling each function by a different name. –  Robert Harvey Jun 24 at 20:18
2  
Naming the functions func1 ... func12 seems insane. At least name them mode1Par2 etc... or maybe myFuncM3P2. –  user949300 Jun 25 at 5:53
2  
" if they modify instead the programmatically-generated file"... create the file only when building from the "Makefile" (or whatever system you use) and remove it right aftter compilation finished. In this way they simply don't have a chance to modify the wrong source file. –  Bakuriu Jun 25 at 11:06

9 Answers 9

up vote 21 down vote accepted

This is a very bad situation, you need to refactor this ASAP - this is technical debt in it's worst - you don't even know how important the code really is - only speculate that it's important.

As to solutions ASAP:
Something that can be done is adding a custom compilation step. If you use Maven that is actually fairly simple to do, other automated build systems are likely to cope with this as well. Write a file with a different extension than .java and add a custom step that searches your source for files like that and regenerates the actual .java. You may also want to add a huge disclaimer on the auto-generated file explaining not to modify it.

Pros vs using a once-generated file: Your developers will not get their changes to the .java working. If they actually run the code on their machine before committing they will find that their changes have no effect (hah). And then maybe they will read the disclaimer. You are absolutely right in not trusting your teammates and your future self with remembering that this particular file has to be changed in a different way. It also allows automatic testing as wel, as JUnit will compile your program before running tests (and regenerate the file as well)

EDIT

Judging by the comments the answer came off as if this is a way to make this work indefinitely and maybe OK to deploy to other performance critical parts of your project.

Simply put: it is not.

The extra burden of creating your own mini-language, writing a code-generator for it and maintaining it, not to mention teaching it to future maintainers is hellish in the long run. The above only allows a safer way to handle the problem while you are working on a long-term solution. What that will take is above me.

share|improve this answer
16  
Sorry, but I have to disagree. You don't know enough about the OP's code to make this decision for him. Code-generating this seems to me like it contains more technical debt than the original solution does. –  Robert Harvey Jun 24 at 20:17
5  
Robert's comment cannot be overstated. Any time you have a "disclaimer explaining not to modify a file" that is a the "technical debt" equivalent of a check-cashing store run by an illegal bookie nicknamed "Shark". –  corsiKa Jun 24 at 20:37
6  
The auto-generated file of course doesn't belong in the source repository (it's not source, after all, it's compiled code) and it should be removed by ant clean or whatever your equivalent is. No need to put a disclaimer in a file that isn't even there! –  Jörg W Mittag Jun 24 at 21:19
1  
@RobertHarvey I am not making any decisions for the OP. OP asked whether it is a good idea to have such templates in the way he has them and I proposed a (better) way to maintain them. It is a not an ideal solution and, in fact, as I have stated in the very first sentence, the whole situation is bad, and a much better way to go forward is to remove the problem, not make a shaky solution to it. That includes properly assessing just how critical is this code, how bad is the performance hit and whether there are ways to make it work without writing a lot of bad code. –  Ordous Jun 24 at 21:58
2  
@corsiKa I never said this is a lasting solution. This would be as much of a debt as the original situation. The only thing it does is decrease volatility until a systematic solution is found. Which will likely include moving entirely to a new platform/framework/language since if you're hitting problems like this in Java - you're either doing something extremely complex or extremely wrong. –  Ordous Jun 24 at 22:02

Is the maintenance hit real, or does it just bother you? If it only bothers you, leave it alone.

Is the performance issue real, or did the previous developer only think it was? Performance problems, more often than not, are not where they are thought to be, even when a profiler is used. (I use this technique to reliably find them.)

So there is a possibility that you have an ugly solution to a non-problem, but it could also be an ugly solution to a real problem. When in doubt, leave it alone.

share|improve this answer

Really make sure than under real, production, conditions (realistic memory consumption, that triggers Garbage Collection realistically, etc), all those separate methods really make a performance difference (as opposed to having only one method). This will take a few days out of your time, but may save you weeks by simplifying the code you work with from now on.

Also, if you do discover that you need all the functions, you might want to use Javassist to generate the code programatically. As other have pointed out, you can automate it with Maven.

share|improve this answer
2  
Also, even if it turns out that the performance issues are real, the exercise of having studied them will help you to better know what is possible with your code. –  Carl Manaster Jun 25 at 0:54

Why not include some sort of template preprocessor\code generation for this system? A custom extra build step that executes and emits extra java source files before compiling the rest of the code. This is how including web clients off of wsdl and xsd files often works.

You'll of course have the maintenance of that preprocessor\code generator, but you won't have the duplicated core code maintenance to worry about.

Since compile time Java code is being emitted, there is no performance penalty to be paid for extra code. But you gain maintenance simplification by having the template instead of all of the duplicated code.

Java generics give no performance benefit due to type erasure in the language, simple casting is used in its place.

From my understanding of C++ templates, they are compiled into several functions per each template invocation. You'll wind up with duplicated mid-compile time code for each type you store in a std::vector for example.

share|improve this answer

No sane Java method can be long enough to have 12 variants... and JITC hates long methods - it simply refuses to optimize them properly. I've seen a speed up factor of two by simply splitting a method into two shorter ones. Maybe this is the way to go.

OTOH having multiple copies may make sense even if there were identical. As each of them gets used in different places, they get optimized for different cases (the JITC profiles them and then places the rare cases on an exceptional path.

I'd say that generating code is no big deal, assuming there's a good reason. The overhead is rather low, and properly named files lead immediately to their source. Some long time ago as I generated source code, I put // DO NOT EDIT on every line... I guess that's save enough.

share|improve this answer

You might want to check whether you can still use some sensible abstractions and use e.g. the template method pattern to write understandable code for the common functionality and move the differences between the methods into the "primitive operations" (according to the pattern descrition) in 12 subclasses. This will mightily improve maintainability and testability, and it might actually have the same performance as the current code, since the JVM can inline the method calls for the primitive operations after a while. Of course, you have to check this in a performance test.

share|improve this answer

There is absolutely nothing wrong with the 'problem' you mentioned. From what I know this is the exact kind of design and approach used by DB server to have good performance.

They have lot of special methods to make sure they can maximize the performance for all kinds of operations: join, select, aggregate, etc... when certain conditions apply.

In short, code generation like the one you think is a bad idea. Perhaps you should look at this diagram to see how a DB solves a problem similar to yours:enter image description here

share|improve this answer

If the function in question is large, turning the "mode/paradigm" bits into an interface and then passing an object that implements that interface as a parameter to the function can work. GoF calls this the "Strategy" pattern, iirc. If the function is small, the increased overhead may be significant... did somebody mention profiling yet?... more people should mention profiling.

share|improve this answer
    
this reads more like a comment than an answer –  gnat Jun 25 at 8:58

How old is the program? Most probably newer hardware would remove the bottleneck and you could switch to a more maintainable version. But there needs to be a reason for maintenance, so unless your task is to improve the codebase, leave it be as it is working.

share|improve this answer
1  
this seems to merely repeat points made and explained in a prior answer posted few hours ago –  gnat Jun 24 at 21:05
1  
The only way to determine where a bottleneck actually is is to profile it - as mentioned in the other answer. Without that key information, any suggested fix to the performance is pure speculation. –  MichaelT Jun 24 at 22:22

protected by gnat Jun 29 at 14:05

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.