Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pattern matcher efficiency train wreck #6686

Closed
scabug opened this issue Nov 18, 2012 · 3 comments
Closed

pattern matcher efficiency train wreck #6686

scabug opened this issue Nov 18, 2012 · 3 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Nov 18, 2012

I can't see a reason f1 shouldn't generate the same code as is generated by f2. Instead, the generated method is over twice as long, the extra effort going to such useful tasks as fetching BoxedUnit to give it a good looking over.

          case10(){
            if (A.this.Foo().unapply(x1))
              {
                val x2: runtime.BoxedUnit = scala.runtime.BoxedUnit.UNIT;
                matchEnd9(true)
              }
            else
              case11()
          };
          case11(){
            if (A.this.Z().unapply(x1))
              {
                val x3: runtime.BoxedUnit = scala.runtime.BoxedUnit.UNIT;
                matchEnd9(true)
              }
            else
              case12()
          };

Test source and bytecode.

class A {
  object Foo { def unapply(x: Int) = x < 0 }
  object Z   { def unapply(x: Int) = x == 0 }
  object Bar { def unapply(x: Int) = x > 0 }

  def f1(x: Int) = x match {
    case Foo() | Z() => -1
    case Bar()       => 1
    case _           => 0
  }
  def f2(x: Int) = {
    if ((Foo unapply x) || (Z unapply x)) -1
    else if (Bar unapply x) 1
    else 0
  }
}

/*** f2: 42 bytes

  0: aload_0
  1: invokevirtual #61                 // Method Foo:()LA$Foo$;
  4: iload_1
  5: invokevirtual #65                 // Method A$Foo$.unapply:(I)Z
  8: ifne          22
 11: aload_0
 12: invokevirtual #67                 // Method Z:()LA$Z$;
 15: iload_1
 16: invokevirtual #68                 // Method A$Z$.unapply:(I)Z
 19: ifeq          26
 22: iconst_m1
 23: goto          42
 26: aload_0
 27: invokevirtual #70                 // Method Bar:()LA$Bar$;
 30: iload_1
 31: invokevirtual #71                 // Method A$Bar$.unapply:(I)Z
 34: ifeq          41
 37: iconst_1
 38: goto          42
 41: iconst_0
 42: ireturn

***/

/*** f1: 86 bytes

  0: iload_1
  1: istore_2
  2: aload_0
  3: invokevirtual #61                 // Method Foo:()LA$Foo$;
  6: iload_2
  7: invokevirtual #65                 // Method A$Foo$.unapply:(I)Z
 10: ifeq          23
 13: getstatic     #30                 // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
 16: astore_3
 17: iconst_1
 18: istore        4
 20: goto          48
 23: aload_0
 24: invokevirtual #67                 // Method Z:()LA$Z$;
 27: iload_2
 28: invokevirtual #68                 // Method A$Z$.unapply:(I)Z
 31: ifeq          45
 34: getstatic     #30                 // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
 37: astore        5
 39: iconst_1
 40: istore        4
 42: goto          48
 45: iconst_0
 46: istore        4
 48: iload         4
 50: ifeq          59
 53: iconst_m1
 54: istore        6
 56: goto          84
 59: aload_0
 60: invokevirtual #70                 // Method Bar:()LA$Bar$;
 63: iload_2
 64: invokevirtual #71                 // Method A$Bar$.unapply:(I)Z
 67: ifeq          81
 70: getstatic     #30                 // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
 73: astore        7
 75: iconst_1
 76: istore        6
 78: goto          84
 81: iconst_0
 82: istore        6
 84: iload         6
 86: ireturn

***/


/*** f1 under -optimise: 62 bytes

 0: aload_0
 1: invokevirtual #55                 // Method Foo:()LA$Foo$;
 4: iload_1
 5: invokevirtual #59                 // Method A$Foo$.unapply:(I)Z
 8: ifeq          16
11: iconst_1
12: istore_2
13: goto          34
16: aload_0
17: invokevirtual #61                 // Method Z:()LA$Z$;
20: iload_1
21: invokevirtual #62                 // Method A$Z$.unapply:(I)Z
24: ifeq          32
27: iconst_1
28: istore_2
29: goto          34
32: iconst_0
33: istore_2
34: iload_2
35: ifeq          43
38: iconst_m1
39: istore_3
40: goto          61
43: aload_0
44: invokevirtual #64                 // Method Bar:()LA$Bar$;
47: iload_1
48: invokevirtual #65                 // Method A$Bar$.unapply:(I)Z
51: ifeq          59
54: iconst_1
55: istore_3
56: goto          61
59: iconst_0
60: istore_3
61: iload_3
62: ireturn

***/
@scabug
Copy link
Author

@scabug scabug commented Nov 18, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6686?orig=1
Reporter: @paulp
Affected Versions: 2.10.0-RC1

@scabug
Copy link
Author

@scabug scabug commented Nov 18, 2012

@adriaanm said:
I guess I should re-calibrate my expectations of DCE and not emit these senseless contemplations of unitness.

@scabug
Copy link
Author

@scabug scabug commented Jan 31, 2013

@scabug scabug closed this Feb 3, 2013
@scabug scabug added this to the 2.10.1 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants