bad java compiler optimization?
I have this piece of code:
private void prepareContent() {
log.info("do something");
// success?
boolean suc = false;
suc = suc || uncompressToContent("file.tar.gz");
suc = suc || uncompressToContent("file.tgz");
for (int i = 0; i <= 9; i++) {
suc = suc || uncompressToContent("dir/" + i + ".tgz");
suc = suc || uncompressToContent("dir/" + i + ".tar.gz");
}
if (!suc) {
log.error("unable to do something");
}
}
The function returns false for "file.tar.gz" and file.tgz"开发者_Python百科.
The problem is the the call to uncompressToContent("dir/1.tgz") returns true and the code stops its execution. The remaining code is not executed.
I'm not sure if this is an error in the compiler. What do you think?
Added: I forgot to mention that I need to execute all the calls to uncompressToContent and check if any returns true, using the fewer instructions as possible.
There is no error in the compiler.
As soon as suc is set to true (i.e. from the first uncompressToContent call) then all of the future expressions will return true without calling uncompressToContent. This is becuase you are using short circuit boolean or ("||") which do not evaluate the second argument if the first argument is true.
If you want all the calls to be made, use the normal or operator ("|") instead.
If the uncompress method returns true if there was a successful decompression, then suc would become true the first time that this happens. Once suc
is true, all the other conditions would be true as soon as suc
is evaluated, so the other part of the OR would not be evaluated. Thus, no decompressions will be attempted once at least one is successful.
This is called short circuiting and is the correct behavior and is a very useful property in most languages. And is also not a compiler optimization since it is part of the defined behavior of the language.
Beyond this answer, there are, I think, ways to make this code more readable. First, are you sure that you want to OR rather than AND here? It seems like you want to quit as soon as one file did not compress decorrectly, not stop as one did decompress correctly.
Second, a better design, IMHO, would be to create a list of all the filenames you want to decompress, and then do a for-each over that list and do all the decompressions, it would make things more readable.
Third, if in most cases decompression would be successful, I think that exception handling is much better than boolean return values.
Here is how I would write something like this (and I would break it into functions)
List<String> filenames = new ArrayList<String>();
this.collectFilenamesToDecompress(filenames) // Write one or more than one functions of this sort based on the semantics of your problem
try
{
for(String filename: filenames)
{
uncompressFile(filename); // This will throw an exception if there is a failure
}
} catch(Exception e)
{
// Announce that there was an error and you stopped decompressing because there was an error.
// Return or quit
}
// If you got here, everything is great!
This behavior is by design.
Logical operators in most languages are short-circuiting.
In the expression a || b
, b
will only be evaluated if a
is false
.
Therefore, once suc
becomes true
, none of the other calls to uncompressToContent
will be evaluated.
I think the compiler is doing something like: suc = uncompressToContent("file.tar.gz") || uncompressToContent("file.tgz") || uncompressToContent("...") || ... So, when it finds one true value, the execution is stopped. Is this feature documented?
Yes. It is clearly documented in the Java Language Specification section 15.24, where it says this:
"The || operator is like | (§15.22.2), but evaluates its right-hand operand only if the value of its left-hand operand is false."
The JLS then goes on to explain exactly what happens in excruciating detail. Follow the link above if you are interested.
Oh yea, and in this respect the Java ||
operator behaves the same as in C, C++, C#, Perl and many other programming languages.
精彩评论