Skip to content

Tweak performance of findRef #9870

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 26, 2020
Merged

Tweak performance of findRef #9870

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2fe5bcf
Tweak performance of findRef
liufengyun Sep 24, 2020
db92078
Add find ref as a bench test
liufengyun Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,15 +503,16 @@ class Typer extends Namer
// Convert a reference `f` to an extension method select `p.f`, where
// `p` is the closest enclosing extension parameter, or else convert to `this.f`.
val xmethod = ctx.owner.enclosingExtensionMethod
val qualifier =
if xmethod.exists then untpd.ref(xmethod.extensionParam.termRef)
else untpd.This(untpd.EmptyTypeIdent)
val selection = untpd.cpy.Select(tree)(qualifier, name)
val result = tryEither(typed(selection, pt))((_, _) => fail)
def canAccessUnqualified(sym: Symbol) =
sym.is(ExtensionMethod) && (sym.extensionParam.span == xmethod.extensionParam.span)
if !xmethod.exists || result.tpe.isError || canAccessUnqualified(result.symbol) then
result
if xmethod.exists then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have thought that the code here would be executed only rarely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a simple program (see below), the speedup is about 24% (34ms/op VS 45ms/op):

class MyInt(val x: Int) {
  def eq(that: MyInt): Boolean = this.x == that.x
}

class Test {
  def foo(x: MyInt, y: MyInt): Boolean = x.eq(y)

  val a = MyInt(2)
  val b = MyInt(3)
  foo(a, b)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I just found out why: The code in question (including the expensive tryEither) is executed twice, once for each MyInt(...). I had not thought of that. So, yes, because of constructor syntax, failures in findRef are now not that uncommon. So this is a definite improvement. Unfortunately, the benchmarks are too noisy to see this immediately.

val qualifier = untpd.ref(xmethod.extensionParam.termRef)
val selection = untpd.cpy.Select(tree)(qualifier, name)
val result = tryEither(typed(selection, pt))((_, _) => fail)
def canAccessUnqualified(sym: Symbol) =
sym.is(ExtensionMethod) && (sym.extensionParam.span == xmethod.extensionParam.span)
if result.tpe.isError || canAccessUnqualified(result.symbol) then
result
else
fail
else
fail
else
Expand Down