dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.57k forks source link

Hash code computation for tear-offs is very inefficient #31875

Open mraleph opened 6 years ago

mraleph commented 6 years ago

This code:

class Foo {
  int tearOff() => 10;
}

void main() {
  final s = new Set();
  final foo = new Foo();
  for (var i = 0; i < 1000000; i++) s.add(foo.tearOff);
}

is roughly 2x slower than this code:

class Foo {
  int tearOff() => 10;
}

void main() {
  final s = new Set();
  final foo = new Foo();
  for (var i = 0; i < 1000000; i++) s.add(() => foo.tearOff());
}

This might be rather suprising for Dart developers. The reason for the slowness is the fact that Closure hash-code for tear-offs is a combination of identity hashcode for the receiver and function itself and neither of those is computed efficiently (see Closure::ComputeHash code)

intptr_t Function::ComputeClosureHash() const {
  ASSERT(IsClosureFunction());
  const Class& cls = Class::Handle(Owner());
  intptr_t result = String::Handle(name()).Hash();
  result += String::Handle(Signature()).Hash();  // (!!!)
  result += String::Handle(cls.Name()).Hash();
  return result;
}

The line marked with (!!!) is extremely expensive - it formats function signature as a string - and it does no caching - so formatting of signature is done every time you invoke f.hashCode on a new tear-off.

It seems that Function::ComputeClosureHash can benefit from some memoization and Instance::IdentityHashCode() can benefit from fast-pathing on 64-bit platforms.

mraleph commented 6 years ago

Tentatively assigning to @ErikCorryGoogle because of the work previously done on hash codes.

lrhn commented 6 years ago

Drive-by comment: Your two examples look quite different since I'd expect the size of the sets to be one vs 1000000. A closure like () => foo.tearOff() can/should create instances that are not equal to any other function instance. A tear-off creates a new instance that's equal to other tear-offs of the same method from the same object.

(I'm guessing you might be optimizing and recognizing that foo doesn't differ even if the scope of the function expression changes between evaluations, and you've done loop-invariant code movement on it. In either case, I'd thing a more comparable example would be:

class Foo {
  int tearOff() => 10;
}

void main() {
  final s = new Set();
  final foo = new Foo();
  tearOff() => foo.tearOff();
  for (var i = 0; i < 1000000; i++) s.add(tearOff);
}

... just to be certain).

mraleph commented 6 years ago

@lrhn good catch. it's much slower than 2x then.