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 inefficiency for basic constructor patterns #6941

Closed
scabug opened this issue Jan 8, 2013 · 4 comments
Closed

Pattern matcher inefficiency for basic constructor patterns #6941

scabug opened this issue Jan 8, 2013 · 4 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jan 8, 2013

The code for all constructor patterns suffers from two inefficiencies. First, there is an unnecessary null-check (note that isInstanceOf will succeed only if scrutinee is non-null). Second, unused fields are still copied into local variables, even if the variable is a wildcard.

Consider this program

object Test {

  def foo(xs: List[Int]) = xs match {
    case x :: _ => x
  }

  def bar(xs: List[Int]) = xs match {
    case xs: ::[Int] => xs.head
  }
}

The two methods should produce equally efficient code. Yet foo takes 48 bytes where bar only takes 34. I have not measured the performance impact of the additional 14 bytes. But in any case 34 vs 48 means that bar can be inlined whereas foo cannot.

Here's the javap output I am seeing:

Compiled from "patmatcode.scala"
public final class Test$ extends java.lang.Object{
public static final Test$ MODULE$;

public static {};
  Code:
   0:	new	#2; //class Test$
   3:	invokespecial	#12; //Method "<init>":()V
   6:	return

public int foo(scala.collection.immutable.List);
  Code:
   0:	aload_1
   1:	astore_2
   2:	aload_2
   3:	instanceof	#16; //class scala/collection/immutable/$colon$colon
   6:	ifeq	40
   9:	aload_2
   10:	checkcast	#16; //class scala/collection/immutable/$colon$colon
   13:	astore_3
   14:	aload_3
   15:	ifnull	40
   18:	aload_3
   19:	invokevirtual	#20; //Method scala/collection/immutable/$colon$colon.hd$1:()Ljava/lang/Object;
   22:	invokestatic	#26; //Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I
   25:	istore	4
   27:	aload_3
   28:	invokevirtual	#30; //Method scala/collection/immutable/$colon$colon.tl$1:()Lscala/collection/immutable/List;
   31:	astore	5
   33:	iload	4
   35:	istore	6
   37:	iload	6
   39:	ireturn
   40:	new	#32; //class scala/MatchError
   43:	dup
   44:	aload_2
   45:	invokespecial	#35; //Method scala/MatchError."<init>":(Ljava/lang/Object;)V
   48:	athrow

public int bar(scala.collection.immutable.List);
  Code:
   0:	aload_1
   1:	astore_2
   2:	aload_2
   3:	instanceof	#16; //class scala/collection/immutable/$colon$colon
   6:	ifeq	26
   9:	aload_2
   10:	checkcast	#16; //class scala/collection/immutable/$colon$colon
   13:	astore_3
   14:	aload_3
   15:	invokevirtual	#49; //Method scala/collection/immutable/$colon$colon.head:()Ljava/lang/Object;
   18:	invokestatic	#26; //Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I
   21:	istore	4
   23:	iload	4
   25:	ireturn
   26:	new	#32; //class scala/MatchError
   29:	dup
   30:	aload_2
   31:	invokespecial	#35; //Method scala/MatchError."<init>":(Ljava/lang/Object;)V
   34:	athrow

}
@scabug
Copy link
Author

@scabug scabug commented Jan 8, 2013

@scabug
Copy link
Author

@scabug scabug commented Jan 8, 2013

@retronym said:
Is this a duplicate of SI-6508?

@scabug
Copy link
Author

@scabug scabug commented Jan 30, 2013

@adriaanm said:
not directly, I think

I have fixes in the pipeline so that both methods are 28 bytes long
under -optimize (exactly the same)

without -optimize, foo is 38 bytes and bar is 34 bytes long,
essentially due to the conflicting requirements of #5158 (store subpatterns in local variables to improve debuggability)

As soon as I have the tests implemented (using Greg's new bytecode testing framework), I'll submit a PR.

@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