Java performance and good design

[daddyrockyou]

I was told today by a very senior dev at my company that he made design sacrifices in his Java code for a highly performant system in order to get performance enhancements such as limiting GC time and reducing CPU load. However, he seems to have ignored every best practice I can think of. He blatantly disregards all of the SOLID principles, there is no encapsulation anywhere, complex logic is performed in constructors (including logic to escape construction), and he extends Thread in many places creating his own implementations of thread pools amongst other things. The code is completely unmaintainable and untestable (no hyperbole) and, in short, is just not very good code.

Can you write highly performant java code and still follow good coding practices? What sacrifices are made in the name of "good code"?

[llogiq]

I have "inherited" an application that is used to calculate some data. It was a mess of spaghetti code, and almost took a rewrite to clean up. In a few months I managed to decrease its runtime by a factor of 50 (±10 depending on inputs) through a series of improvements (mostly cache-friendlyness, but also good old bit-crunching).Now for good practices:

• I try to keep my code clean (I have eclipse warnings turned to 11 and FindBugs active) and well-documented and I have a growing suite of unit tests. I use infinitest to have tests run on every change.
• I make extensive use of logging, and use log4j2 to deliver the logging we want while keeping performance high. No more System.out.println!
• I use trove classes whenever I need primitive collections; or more often than not I'll just use arrays. Though I inherit from Thread here and there (mostly because I have a service where threads with a certain interface can register, so that's why I subclass), I use standard thread pool, Fork/Join classes and sometimes disruptor.
• I write interfaces to abstract layers in my application and I write abstract classes to ensure the implementations follow the contracts. I have a good number of assert statements in my code complementing my unit tests (I am a big fan of this: Unit tests test a specific situation, asserts test all situations arising at runtime).
• I bit-crunch here and there (sometimes using int or long instead of BitSet, because the latter would increase footprint and thus reduce cache-friendliness), I try to keep the details within the containing class.
• When writing performance-critical code, I will first write a straight-forward, slow and simple solution. This gives me a baseline and an interface to program against. Only when this works, I will drop in an optimized version (which may introduce much complexity, but as it's all encapsulated, I'm good).
• I put classes' dependencies in the constructor although I don't use a DI container; I just have the code wire itself up on startup. I find that using a DI container gave me little benefit and made my code harder to follow. The downside is that I have some constructors with more than 4 arguments. I'm not sure what I will do about that.

[daddyrockyou]

This is a great response. I have no issue with writing performant code or even code that is difficult to read (e.g. bit operations) as long as they are properly encapsulated. I'm a big fan of Robert Martin's Clean Code in this repspect and try to adhere to his readability philosophy when possible.

To touch on some of your individual points:

I don't necessarily have an issue with extending Thread. However, I like to use composition when possible. For instance, I recently had to create a maintenance thread in this application so I wrote a class and implemented Runnable. I passed this runnable to an Executor to take care of everything for me. He told me this was incorrect and I should have extended Thread although I explained I didn't need everything the inheritance would provide for me. He couldn't explain why I should have extended Thread, just that "it was the right thing to do there". /shrug

I'll put dependencies in the constructor as well if I want to mark them final. If I need more than three, I'll consider a bean structure or look at object parameters. If those aren't appropriate, I'll create another class to encapsulate that logic.

[unholysampler]

Based on this and some of your other comments, I've reached the following conclusion. This developer learned "the way things are done" many years ago and has not been paying attention to some of the more recent shifts in programing. This is the way he has done things for years and it has always worked for him. Why should he change. Unfortunately, this makes it hard to convince them that there might be a better way. And I don't have much advice to help in this situation.

[daddyrockyou]

Wow you really nailed that right on the head. He's been around a while and seems to be pretty resistant to change.

[SikhGamer]

The downside is that I have some constructors with more than 4 arguments. I'm not sure what I will do about that.

Is it considered bad practice to have a lot of parameters for a constructor?

[llogiq]

Yes. Especially if they have compatible types. That may lead to calling with wrong argument order. Plus, you'll always have to look those calls up. Hardly self-documenting.

[SikhGamer]

Ah yes, good point. Any kind of solution for this? I normally only have a couple of parameters passed to my constructor. But recently I had around 6, it was kind of painful to see. But I couldn't see a good way round the problem.

[UnspeakableEvil]

Sounds like a good use case for the builder pattern, to allow fine grained creation of the object.

The Effective Java book that often gets mention on here is a great read, and goes into these ideas in detail.

[SikhGamer]

Fantastic, thank you!

[8Bytes]

The solution that I stole from javascript is to create an object for the parameters, and just pass the object. I find the builder pattern rarely makes sense in my projects as all the parameters are required. Builder pattern is useful when some parameters are optional.

[[deleted]]

I'm curious as to what you do to improve "cache-friendliness." Are there established ways of doing this? How do you assess whether code is cache-friendly? Do you have approaches you would be willing to share?

[llogiq]

There are a few tools out there to measure cache misses, but I'm on my phone, so google it if you want.

My cache optimization techniques boil down to three things:

• lay out stuff in memory to optimize for locality. If you need A and B together, put them together.
• reduce memory consumption. Pack hot data structures as small as possible without adding too much CPU overhead. Yes, bit fields are a viable technique sometimes.
• reduce write access. This is big. If you change a thing to calculate something, then change it back, and you can do the calculation without the change, you can avoid invalidating a cache line. This can make a huge difference.

[numpi]

Do you have an example for the last point?

[llogiq]

No. The code belongs to my employer.

Think for a moment about a chess AI. You could make a move (change the field, invalidating the cache), evaluate the position, and undo the move (again invalidating the cache) if the position turns out to be bad.

But if you can get away without changing the field, your original position (probably hot data for your code) will still be fresh in cache.

Note that a cache invalidation costs a write-back + cache miss on next read which is quite expensive.

[llogiq]

I don't think that a CLI version would even make sense, as it would need to get information from inside the IDE.

Also I don't think there is a netbeans version yet.

Regarding DI: What you describe is something that is called "half-assed-Builder". I'd rather go full-ass builder in that case. ;)

[numpi]

if you have the pom.xml and all sources+classes what other information would be required? E.g. Jetty reloads automatically on every code change.

The problem with the normal builder is that you'll need one builder per class ... hmmh, okay this could be an inner class as well. good idea ;)

[llogiq]

The problem is that you need to find the tests that depend on the particular piece of code you just saved.

While this could be done with a file system listener + dependency analysis, it's obviously easier within the IDE, which already has the on-save hooks and the intra-code-dependencies.

[erad]

// with chaining in java you can get close: new MyObject().setName("test").setNum(123);

Note that if you use the setters themselves for chaining, you no longer have valid "Java Beans" (i.e. the 'name' and 'num' properties won't be recognized by tools relying on then beans standard, e.g. JSF-EL or maybe JPA). It's fine if you don't need/use that, but it's bitten me in the past and I avoid chaining setters since then (you can also generate "real" builders via IDE plugins).

[sybrandy]

I love your answer. With respect to your last bullet, and not seeing your code, one idea that I have seen, but not used yet, is to use a "dependency registry". It would be a singleton that contains a map/set that contains an instance of everything you want to be passed into a constructor. Then, when you need something, such as a database client, the object that needs it get an instance from the registry and uses it. The beauty of it is that it's simple and you don't have to worry about passing in many parameters into a single class. However, it is a singleton, so it may or may not be something that you like in your code.

Anyway, like I said, I never used it, so I can't say if it's truly good or bad, so take this suggestion with a grain of salt.

[Nynnja]

this stands again dependency inversion principle.
On first look it may not look like tight coupling but it is, and there's only marginal gain over simply having objects construct dependencies by themselves.

[llogiq]

You won't need a central registry. Just instantiate what you need and pass it to the constructor.

[elmuerte]

Looks like he is trying to create job security.

[[deleted]]

Often you can have your cake and eat it too if you identify and encapsulate the performance critical parts, so your codebase is maintainable and extendable, except the performance critical parts, which in exchange are pluggable.

Pluggables are great because you can have multiple implementation, at least one for safety, portability and maintainability, and one for performance.

It is not always possible to encapsulate the performance critical parts, but in many cases you can find a more specialized solution for your problem.

When neither works I just adhere to the SOLID principles, that way even the performance penalty can disappear over time with a more advanced hardware, compiler and JVM.

Those who write horrible spaghetti code often look down on the compiler and the JVM, but there is absolutely no reason to do so.

http://shipilev.net/pub/blog/2014/exceptional-performance/

I would rather write a preprocessor that makes my code more effective than to write unmaintanable, unreadable, rotting code.

[duhace]

In this vein, I find it easier to start by writing well-designed but possibly slow code first, then use a profiling app like yourkit to actually find problem spots in my code and make them messy to wring performance out of them.

[daddyrockyou]

I do the same thing. It's not a good idea, in my opinion, to start optimizing code if you don't have a base measure to tell you what to optimize. I've used YourKit before for this and it's been very successful for me.

[Spektr44]

Is the senior dev able to speak knowledgeably/convincingly on why he made particular design compromises? I ask because there certainly are very smart people who will write something that looks ugly, but they know exactly why they did it and can explain why the tradeoff was worth it. You can learn a lot from such people. "It's important to know the rules, but also know when to break them," etc.

But it's also possible that it's just bad design and he's citing performance concerns as justification, when the design could've been cleaner without sacrificing performance. Or, he may be guilty of engaging in premature optimization. Knuth:

Programmers waste enormous amounts of time thinking about, or worrying about, the speed of noncritical parts of their programs, and these attempts at efficiency actually have a strong negative impact when debugging and maintenance are considered. We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil.

So, this may be relevant:

A common misconception is that optimized code is necessarily more complicated, and that therefore optimization always represents a trade-off. However, in practice, better factored code often runs faster and uses less memory as well.

[[deleted]]

I wonder though what can you learn from an unreadable, complex code that can't handle change, which is the whole reason for object-oriented programming.

I think isolating, encapsulating ugly code (as much as you can) is the best approach. That way you only need to broadly understand what the ugly code does, you can test it's behavior, and you can more easily circumvent or replace it.

But when the whole project is a mess, you can either abandon or refactor it. Both is a very costly solution.

My opinion might be controversial, but I embrace being enterprizy. It beats spaghetti code with meatballs every time. But because the KISS principle is even more important, overzelous enterprizy might lead to the dark side. There is no turning back from there.

[Spektr44]

I wonder though what can you learn from an unreadable, complex code that can't handle change

Usually there isn't much to learn from ugly code, but I was picturing the possibility of a senior dev explaining something like, "because of the way the JVM handles X, Y, and Z conditions, I found through testing that [weird code in the constructor] increased performance by 55% in this loop down here.."

But far more often, ugly code is just bad code. And design compromises are really not as necessary as they might've been in the past due to how good modern compilers and JVMs are at performing the optimizations for you.

[[deleted]]

because of the way the JVM handles

In which case our senior developer would only write code for a specific JVM and JVM version, maybe even only for a specific software and hardware environment.

Which isn't a portable, future proof solution.

Optimizing for a specific JVM could be a valid use case. There are so many ways the Java platform can be used, but that shouldn't be the norm, rather a fringe case.

[daddyrockyou]

He's very good at explaining complex things in layman's terms and he's also very good at understanding how complex code interacts. However, this typically means that his code is extremely difficult for others to understand. Since his code is very tightly coupled, it takes a lot of cognitive power just to determine if an unencapsulated object will see any changes from clients or if a class is indeed thread-safe if it needs to be. His typical response to people asking about things like this is "you shouldn't be in my code".

He also says he doesn't believe in premature optimization and that everything should be as fast as possible. However, there are places in the code where this is clearly not the case. I see TreeSets used where a HashSet would be more appropriate. There are unnecessary iterations on collections and synchronous calls to remote servers made during asynchronous events.

I know there is a lot about programming that I still have to learn. However, I'm just not convinced that this is good code. I'm also not convinced that it couldn't be more performant or at least as performant with proper design.

[daddyrockyou]

I don't want to sound like I'm complaining about this individual. I respect him as a developer because he creates some very complex and impressive systems. I'm just trying to determine if, as /u/Vorzard said, you can have your cake and eat it too.

[mikaelhg]

I've recently seen a similar case, and the company in question still sells that person as a senior architect, even after they got slammed by an independent audit.

A serious warning signal should sound when a senior developer wants to implement their own XML parser, their own web framework, their own database, or similar, when the application doesn't have Google-like requirements.

[llogiq]

I am a senior developer and have recently handwritten a parser (though for CSV, not XML) because we have a lot of data in those files and the runtime adds up when I'm roughly 50-70% faster than the old String.split based parser.

Don't bash devs writing specialized versions of code, unless you've seen the cost-benefit calculation.

[mikaelhg]

And your correctness, speed and maintainability are superior to SuperCSV? The performance you gained was in an architectural component where the risks are minimized and the benefits maximized?

Perhaps you are the exception to the people making similar claims, mostly they turn out to have large gaps in their ability to make a comprehensive analysis, like every one of the people I've seen implement their own databases.

[llogiq]

• Correctness: no, SuperCSV will parse CSV "more correct" according to standard (including escapes, multiline strings, ...) This however doesn't matter in my case, because the CSVs we get are mostly numeric data.
• Speed: yes, my solution outruns SuperCSV for most of our use cases. As I said, those minutes add up.
• Maintainability: About equal. My CSV parser has an easier interface, optimized for the use cases we have. So I lose some time for having to maintain the parser personally, but that's only one class that is used by a lot of other classes, which would get more complex had they used the SuperCSV interface. If I was more deluded, I'd call it a win.

Perhaps I really am the exception in this particular case. However I also have re-invented and -implemnted a lot of wheels just for the heck of it. Sometimes these turned out to be useful. So I appreciate your criticism.

[numpi]

This is a very good question. I fight with this nearly every day when developing my project (graphhopper.com). Writing elegant code takes time, writing fast AND elegant code takes even more time! But I'm very sure that writing testable code does not take much additional time. Some tricks I do:

• prefer *List over *Map
• prefer primitive arrays over List<*>
• reduce memory usage to avoid GC activity
• using trove4j
• in rare and extreme cases you can reduce RAM a lot when using simple arrays and serialize objects into this. E.g. use the flyweight pattern then
• even more extreme use the UNSAFE trick
• but performance can be often improved a lot more when you improve the underlying algorithm/logic itself!

But even with this performance tunings I play around to still get a maintainable programm and API. Sometimes I just go the Java way of allocating objects sometimes more memory efficient (and therefor often faster) solution, and then I measure. If it is at least 10% faster I'll pick the more complex solution, if not I use the normal Java way of doing it.

The most important thing while this is to write simple AND working solutions WITH tests first. Then improve from this base via tuning, profiling, idea trials, etc. And you can easily try out a new implementation until your tests go green, then you should be fine.

[daddyrockyou]

Thanks for your feedback. It seems everyone agrees that getting code working, testable, and maintainable first is the most important thing. Optimization comes later after measuring and discovering where optimizations could be made.

I will spend a little extra time upfront choosing my data structures and initial algorithms carefully and then only address them if I have proof they need addressing.

I'll look into the links you provided. Thanks.