History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: JANINO-57
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Arno Unkrig
Reporter: Andrzej Walczak
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Janino

Switch statment compiles space inefficiently for a large case span.

Created: 09/May/06 02:20 AM   Updated: 26/May/06 06:42 AM
Component/s: None
Affects Version/s: None
Fix Version/s: None

Time Tracking:
Not Specified


 Description  « Hide
Observation:
Switch satements are compiled space inefficiently for lare case spans. E.g:
//--------------------------------
switch(var) {
case -10000: ....; break;
case 10000: ....; break;
}
//---------------------------------
Compiles to a large amount of bytecode. For larger spans of cases the class is beyond the 64k limit and fails to be loaded into jvm.

Assumption:
Large switch statements are compiled only to the linear switch opcode (tableswitch).
The mapped version of switch (lookupswitch) is not used.



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Arno Unkrig - 16/May/06 03:32 PM
Can't reproduce. In my test environment,
Bug_57.java
public class Bug_57 {
    public static void main(String[] args) {
        int x = 17;
        switch (x) {
        case -10000:
            System.out.println("HELLO");
            break;
        case 10000:
            System.out.println("WORLD");
            break;
        }
    }
}

compiles to a class file of 517 bytes.

What version of JANINO are you using? What is the precise content of your source file? How do you compile it (Compiler, ScriptEvaluator, ...) ?


Don Schwarz - 24/May/06 08:47 PM
I ran into this problem as well. I can't reproduce it with Andrzej's test case, but here's one equally simple where I can:

public class Bug_57
{
public static void main (String[] args)
{
switch (Integer.parseInt(args[0])) { case Integer.MAX_VALUE: System.err.println("HELLO"); break; case -1: System.err.println("WORLD"); break; }
}
}

It seems that Janino is using the right algorithm to choose between TABLESWITCH and LOOKUPSWITCH. Unfortunately, it's using an int to store the range, so when the difference between the smallest key and the largest key exceeds 2**31-1, it overflows and makes the wrong decision. Ironically, a very large range is the worst possible time to choose TABLESWITCH, hence the very large class files.

The easiest fix is just to use a long to hold the range. Here's a patch against Janino 2.4.3:

— janino-2.4.3/src/org/codehaus/janino/UnitCompiler.java 2006-05-11 23:05:12.000000000 -0500
+++ janino-2.4.3-fixed/src/org/codehaus/janino/UnitCompiler.java 2006-05-25 04:32:37.000000000 -0500
@@ -762,10 +762,11 @@

// Generate TABLESWITCH or LOOKUPSWITCH instruction.
CodeContext.Offset switchOffset = this.codeContext.newOffset();

  • if (!caseLabelMap.isEmpty() && 2 * caseLabelMap.size() >= (
  • ((Integer) caseLabelMap.lastKey()).intValue() -
  • ((Integer) caseLabelMap.firstKey()).intValue()
  • )) {
    + long max = 2 * (long)caseLabelMap.size();
    + long range = (long)((Integer) caseLabelMap.lastKey()).intValue() -
    + (long)((Integer) caseLabelMap.firstKey()).intValue();
    + if (!caseLabelMap.isEmpty() && max >= range)
    + {
    int low = ((Integer) caseLabelMap.firstKey()).intValue();
    int high = ((Integer) caseLabelMap.lastKey()).intValue();

It seems that the CVS archive is not responding, so I couldn't be sure this is the latest version.


Arno Unkrig - 26/May/06 06:42 AM
Fixed it as propsed. Also added an optimization for SWITCH without CASE labels.