Oh JavaFX, why, why, why?

Correction

As noted by one of the comments on this post. The code I mention below only relates to files not urls. This negatives my statement about slow servers. My apologies for the misrepresentation, I should have been more careful.

If you’ve ever done any programming you’ve probably had to deal with I/O (Input/Output) at some point. Read a file, open a url, open a database connection. It’s inevitable that you interact with other systems via I/O in some way or another if you want to achieve something useful.

I’ve recently been trying to nail down a performance issue I noticed with opening a menu in JavaFX. The first time it opens it displays almost instantly, the second time there is a noticeable delay, at least a quarter of a second.

Obviously this shouldn’t be happening but after checking my code there was no roadblock that could be causing the delay. Firing up the excellent YourKit Java Profiler (they gave me a year’s license for free!) showed that on the second opening of the menu the stylesheets for the application are being reloaded and a digest (a hashcode) is being generated to determine whether it has changed (more on why this is a bad idea later).

But the most puzzling issue are the following lines of codes (taken from StyleManager):

try (final InputStream stream = url.openStream();
     final DigestInputStream dis = new DigestInputStream(stream, MessageDigest.getInstance("MD5")); ) {
   dis.getMessageDigest().reset();
   while (dis.read() != -1) { /* empty loop body is intentional */ }
   return dis.getMessageDigest().digest();
}

If you’ve ever done any Java coding before and dealt with streams I’m sure you can spot the problem immediately.

Not seeing it?

This is the problem line:

DigestInputStream dis = new DigestInputStream(stream, MessageDigest.getInstance("MD5"));

The “stream” variable is not wrapped in a BufferedStream. This means that the stream is reading a single byte from the stylesheet (file or url) then adding that to the digest. This single problem is making the digest code take at least 10 times longer than it should. But if I change the code to add a BufferedStream around the stream, thus:

try (final InputStream stream = url.openStream();
     final DigestInputStream dis = new DigestInputStream(new BufferedStream (stream), MessageDigest.getInstance("MD5")); ) {
   dis.getMessageDigest().reset();
   while (dis.read() != -1) { /* empty loop body is intentional */ }
   return dis.getMessageDigest().digest();
}

the time taken to read and digest my stylesheet drops from 270ms to 15ms.

It took me a while to come to grips with this since it is such a rookie mistake. This is the type of mistake that new grads fresh from University make in the first six months of their first job. It’s naive code, an easy mistake to make when you don’t know better.

Buffering is so fundamental to how streams and Java I/O works that it has its own section in the Java tutorial. The first line of that page reads:

Most of the examples we’ve seen so far use unbuffered I/O. This means each read or write request is handled directly by the underlying OS. This can make a program much less efficient, since each such request often triggers disk access, network activity, or some other operation that is relatively expensive.

https://docs.oracle.com/javase/tutorial/essential/io/buffers.html

This tells me a number of things about JavaFX.

1. They aren’t doing code reviews. There is no way that this rookie error would pass a code review (in my first job at Nortel Networks we tried hard to be ISO 9000 compliant and punishing, detailed code reviews, for every line of code you produced, by a team of senior colleagues was normal). Unbuffered streams are one of the first no-nos you learn when coding Java I/O. If even a single pair of other eyeballs had seen this code it would be flagged.
2. The JavaFX developers don’t care about performance. Clearly no one working on JavaFX is doing large scale performance testing of JavaFX. It’s obviously being tested but not at scale and performance doesn’t seem to be a consideration. No one has ever asked the question, is this causing a performance issue?
3. The base layers of JavaFX were clearly rushed and haven’t been revisited to be improved. This sort of issue should not be occurring in production code that is released to others. We all take shortcuts, it’s inevitable in larger systems, but it’s clear that a lot of things have been rushed and not revisited in JavaFX. For example, why hasn’t this code been updated to use the java.nio.file package? It was introduced in Java 1.7 in 2011. Why hasn’t JavaFX moved to the infinitely superior java.nio.file?

The bigger problem

I mentioned earlier that JavaFX is creating a digest of the stylesheet (each stylesheet in fact) and that it is a problem. To me this is the bigger problem, the performance issue is annoying (and bewildering) but JavaFX creating the digest is the real issue because it is deciding that if the stylesheet has changed then it should re-apply it.

This is stupid for a number of reasons. The main one is that I can’t find anywhere that this behaviour is described and frankly it’s very unwelcome behaviour. If I set a stylesheet in my application it is up to me and me alone to change it. Otherwise the UI may change at some random time, a time not triggered by me.

Consider, JavaFX allows you to use a URL to reference a stylesheet. Let’s say I decide to use a CSS file on a remote server, for example, http://myserver.com/css/myapp.css I may not have control of that file and so if it changes it might change my application and at odd times. The triggering of whether the file should be re-applied is made when JavaFX decides that a stylesheet check is required not me.

Also, since when would anyone want their application to change, without notification, if a file changes? For the http example I gave, what happens if the server is slow or has a huge delay in returning the file? Combined with the code reading one byte at a time it could minutes to update the UI with the changed stylesheet.

Again, why did the JavaFX developers think this was a good idea?

Captain Picard is facepalming so hard right now…