When will Sun’s JDK start following code conventions?
A lot of Java applications run on Sun’s JDK due to various reasons. In fact till recently Sun had a very restrictive license policy for Java. So, effectively Sun’s JDK has been a de facto standard. That’s the reason Sun’s JDK code is supposed and expected to be of high quality and following at least the coding conventions recommended by Sun in a 1999 article that is widely used.
If you look under the hood of JDK, you will find a completely different picture. Here is an example from the Java 6 source code of StringBuilder class. (I have noticed similar stuff in 1.4 and 1.5 releases.)
How many non-conformance instances can you find? Here is what I notice:
- Indentation - not sure if it was edited in notepad (even vi does indentation) or a result of source control merge?.
- If clause doesn’t use brackets- Here is an excerpt from Sun’s Java coding conventions.
Need I say more?
- Creating “null” string instance- Why can’t it be represented by a static final constant and reused? All the string literals are stored in JVM in a String pool, even the ones defined as constants. Isn’t it more optimized if you have a direct reference to it rather than JVM doing a lookup in the pool every time you call the method?
Note: After looking at the bytecode generated by the compiler, it looks like a direct reference is pushed on the stack for the string literal. So, it effectively doesn’t matter if you use a literal or a constant. Still, if you are using a string literal over and over again, it may be a better idea to create a constant. This way if you need to change the string literal, there is only one place where you need to make change. So, you protect yourself against future changes. Thanks to Messi for pointing this out in the comments :). - Variables can be made final-The method parameter
sband local variableslenandnewcountcan all be declared final for optimization. This article has good explanation for it.
Note: Again after analyzing the bytecode it seems like compiler really doesn’t care about the final declaration for the local variables. The method parameter are a different case as by declaring them final you make sure that they are not modified by mistake. Also, the local variables can be declared as final as a coding practice to mark them as immutable even though it doesn’t affect the byte code. But it makes it clear for a person looking at your code that the variable can not be modified. Thanks to Messi and Kevin Riff for pointing this out in the comments
This is just an example. There are instances like this all over the JDK code base. I want to make it clear that I am not commenting on the performance aspect of JDK, just the code conventions and formatting (for the obvious reasons that the code should be properly formatted). Now, since Sun has released the JDK source code under GPL license, do we expect to see things like this continue? Do you think people will take more responsibility and clean up the JDK code? I hope so. I also hope that Sun does not blacklist me after this post
Toolman said,
December 12, 2006 @ 7:01 pm
about your 3rd point: Strings delcared without the new prefix are actually pooled and reused - sure its not static or final, but it is ‘reused’
Atif Khan said,
December 12, 2006 @ 7:27 pm
Aren’t the strings in pool referenced by a hash? So, why would you not instead define a static constant for it? Wouldn’t that be more optimized? It will be interesting to know how exactly this works.
messi said,
December 12, 2006 @ 10:09 pm
1) Is it possible that you set your tab width to zero? Sun uses four spaces for indentation and a tab for every eight spaces.
2) Your excerpt from Sun’s Java coding conventions is not about missing braces. It’s about placing bracing and “else [if]” on the same line.
3) String literals are already (private static final) constants but with no name. See String.intern() for more about the VM string pool.
4) Compilers can see if you reuse a local variable and optimize code. There’s no need to mark it “final”. At least not for optimization.
5) The Classlib is not (yet) released under the GPL.
ujjwal said,
December 13, 2006 @ 1:28 am
“2) Your excerpt from Sun’s Java coding conventions is not about missing braces. It’s about placing bracing and “else [if]” on the same line.”
is it some sort of javaesque assumption ?? Even if it is not about missing braces then why the note below ??
Atif Khan said,
December 13, 2006 @ 10:39 am
Messi,
Good points indeed. I will try to address them in the sequence:
1) I don’t think it’s an issue with my tab spacing. I have opened the file in Notepad, Wordpad, Eclipse, TextPad and vi with the same results. On a personal note, I think all the tabs should be converted to spaces to avoid issues like these. That’s just my opinion.
2) I am not sure if you looked at the excerpt or you are just interpreting it differently. It’s mentioned 2 times in there that you should use the brackets.
3) I understand that all the string literals are stored in pool, even the one defined as constants. Isn’t it more optimized if you have a direct reference to it rather than JVM doing a lookup in the pool every time you call the method?
4) This is from the article I reference in the post: “Declaring a final field helps the optimizer make better optimization decisions, because if the compiler knows the field’s value will not change, it can safely cache the value in a register. final fields also provide an extra level of safety by having the compiler enforce that a field is read-only”. So really, why would you not do it? It’s better to enforce things programmatically than to depend on compiler for all the optimizations.
Jonathan Allen said,
December 13, 2006 @ 2:19 pm
Screw coding standards, what about correctness?
Why should appending a null object append a string with the value “null”?
When java is internationalized, will it append “falta de información” instead?
It makes no sense!
It should have either thrown an exception or silently failed.
messi said,
December 13, 2006 @ 2:33 pm
1) Just a guess. I checked java/lang/StringBuilder.java in src.zip from JDK5 and JDK6 and both methods contain tabs.
2) Sorry, you’re right. I didn’t scroll down to the //AVOID comment example. I’m glad Sun doesn’t follow that convention.
3) String constants are loaded by a the const_n bytecode instruction which means directly pushing the reference onto the stack. No lookup required.
4) field != local variable
messi said,
December 13, 2006 @ 2:45 pm
3) “ldc” is the instruction’s mnemonic.
Atif Khan said,
December 13, 2006 @ 4:11 pm
Messi,
Thanks for the clarification. I do see the bytecode instruction for the string constant being pushed on the stack. Now this raises some questions:
1). Since the compiler generates a “ldc” for loading the reference to the string literal, is there any value in defining a constant even if the literal is used several times? The only cases I can think of is for programmatic accuracy and having a single point for future changes. Can anyone think of something else?
2). I understand “field != local variable”. But as far as the memory allocation for two, wouldn’t they both benefit from “final” declaration as they will be marked as immutable?
3). I do like the brackets and spaces instead of tabs. But, that’s just my personal preference
Kevin Riff said,
December 13, 2006 @ 4:45 pm
All compile-time constants (which includes strings and numerical values) are inlined by the compiler. All string constants, whether they are literals or not, are encoded with the ldc instruction by the compiler. The first time that ldc is encountered there may be a slight delay to resolve the constant pool entry at that location, but after that it’s as fast (or faster) as an array lookup. So feel free to create a constant if you want to define the string’s value in one place, but it makes no difference to the way it’s referenced in the byte code.
The ‘final’ modifier on local variables only has an effect at compile time. In fact, it isn’t even possible to tell the JVM which local variables are final. That’s not surprising really if you realize that even the type of local variables (not parameters) is not encoded in the class and must be inferred by the JVM based on the bytecodes used. Some coding standards require the use of ‘final’ to prevent accidental modifications, but it has absolutely no effect on the compiled code.
Atif Khan said,
December 15, 2006 @ 10:36 am
Messi and Kevin,
I have updated the post with your inputs. Thanks for the contribution.