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

"Tearing off" a method is extremely slow #551

Closed
DartBot opened this issue Nov 21, 2011 · 6 comments
Closed

"Tearing off" a method is extremely slow #551

DartBot opened this issue Nov 21, 2011 · 6 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.

Comments

@DartBot
Copy link

DartBot commented Nov 21, 2011

This issue was originally filed by rice@google.com


The following code times two operations:

  1. call a closure that calls a method 'tearOff' on a particular object
  2. call a closure that returns a closure representing the object's 'tearOff' method, then call that closure

On my machine (Mac Pro, OS X 10.5.8) the second approach is about 1700 times slower in the VM.

class Test {
  Test();
  void tearOff() {}
}

class Benchmark {
  Test test;
  Function closure1;
  Function closure2;

  Benchmark();

  void run() {
    test = new Test();
    closure1 = () => test.tearOff();
    closure2 = () => test.tearOff;

    for (int iter = 0; iter < 10; iter++) {
      bench();
    }
  }
  
  void bench() {
    Stopwatch w1 = new Stopwatch();
    w1.start();
    for (int i = 0; i < 10000; i++) {
      closure1();
    }
    w1.stop;
    double t1 = w1.elapsedInUs() / 10000;
    
    Stopwatch w2 = new Stopwatch();
    w2.start();
    for (int i = 0; i < 10000; i++) {
      closure2()();
    }
    w2.stop;
    double t2 = w2.elapsedInUs() / 10000;

    print("$t1 $t2 ${t2 / t1}");
  }
}

void main() {
  new Benchmark().run();
}

@rakudrama
Copy link
Member

The first loop contains one function call per iteration.
The second loop contains two function calls and the allocation of a closure (the tear-off) per iteration.
I measure a ~320x hit for the extra call and allocation - that seems high (r1815, Goobuntu)
I measure a ~45x hit on Frog.
But the loops are not doing comparable work so it is hard to interpret these results.

To make the loops comparable, closure1 should also return a closure:

    closure1 = () => () => test.tearOff();
    ...
      closure1()()

With this change I measure 180x hit. So creating tear-offs really is terribly slow.
(Frog is 1.6x, so Frog's tear-off creation is slower than a hand written closure too, but not disastrously so.)

Another interesting comparison is to make the loops comparable the other way:

    closure2 = test.tearOff;
    ...
      closure2();

This creates a tear-off once and calls it many times.
With this I get that the VM is at 0.6x, i.e. nearly twice as fast. Calling a tear-off is great.
(OTOH, Frog's tear-offs are about 4x slower to call, but that can be reduced easily to ~ 1.2x)


cc @jmesserly.

@whesse
Copy link
Contributor

whesse commented Nov 23, 2011

cc @peter-ahe-google.

@rakudrama
Copy link
Member

Issue #583 contains more details of a suggestion for Frog.
I suggest we track VM in the issue and Frog in Issue #583.

@iposva-google
Copy link
Contributor

Added this to the Later milestone.

@peter-ahe-google
Copy link
Contributor

I'm not sure if this is still a problem. However, if it is, would it be possible to add an option to help us find performance problems related to this?

@mraleph
Copy link
Member

mraleph commented Jan 18, 2013

Fixed by r17261


Set owner to @mraleph.
Added Fixed label.

@DartBot DartBot added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. labels Jan 18, 2013
@DartBot DartBot added this to the Later milestone Jan 18, 2013
nex3 pushed a commit that referenced this issue Aug 31, 2016
copybara-service bot pushed a commit that referenced this issue Jul 20, 2023
…string_scanner, test, test_descriptor, vector_math

Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/06d7288..a04ac3e):
  a04ac3e8  2023-07-19  Sam Rawlins  Format testing and fix static-type-parameter issue (#3465)
  a44ecebc  2023-07-19  Sam Rawlins  Move validation-related tasks to package:args (#3463)
  392812e4  2023-07-17  dependabot[bot]  Bump analyzer from 5.13.0 to 6.0.0 (#3458)
  642e8d8b  2023-07-17  Sam Rawlins  Convert 'p' prefixes to 'path' in test/ (#3462)

ecosystem (https://github.com/dart-lang/ecosystem/compare/2052a4c..27ff3e9):
  27ff3e9  2023-07-19  Moritz  Add debug message (#142)
  02703ce  2023-07-18  Moritz  Split commenting into new workflow (#141)
  a52ac63  2023-07-18  Moritz  Finish renaming `coverage_web` (#140)
  ffa1ecb  2023-07-17  Moritz  Health workflow updates (#134)

leak_tracker (https://github.com/dart-lang/leak_tracker/compare/515612e..098bafc):
  098bafc  2023-07-20  fzyzcjy  Update pubspec.yaml (#75)
  7606fd1  2023-07-19  Polina Cherkasova  Improve performance. (#98)
  e6778b3  2023-07-18  Polina Cherkasova  Explain case when static object causes leaks. (#96)
  cca1d77  2023-07-18  dependabot[bot]  Bump actions/checkout from 3.5.2 to 3.5.3 (#86)

markdown (https://github.com/dart-lang/markdown/compare/ee4e1b3..faabb1a):
  faabb1a  2023-07-17  Kevin Moore  Bump pkg:http (dev) dependency (#551)

mime (https://github.com/dart-lang/mime/compare/bdb66bd..799b398):
  799b398  2023-07-18  Kevin Moore  update lints, require Dart 3 (#101)
  d17e3ed  2023-07-18  Kevin Moore  blast_repo fixes (#100)

native (https://github.com/dart-lang/native/compare/7c474e1..2598ac6):
  2598ac6  2023-07-20  Daco Harkes  Package name spacing (#101)
  3d73b4a  2023-07-19  Daco Harkes  [native_assets_builder] Builder `out/` folder (#99)
  6308330  2023-07-19  Daco Harkes  Remove compiled files (#92)

string_scanner (https://github.com/dart-lang/string_scanner/compare/35657e2..413b57a):
  413b57a  2023-07-18  Kevin Moore  Require Dart 3, update lints (#61)
  53690da  2023-07-18  Kevin Moore  blast_repo fixes (#60)

test (https://github.com/dart-lang/test/compare/a92b5bb..37e54e3):
  37e54e32  2023-07-19  Nate Bosch  Handle initial message entirely within conditional (#2067)
  e76bffe8  2023-07-19  Nate Bosch  Expand dom interop to cover more uses (#2066)
  8bc188f7  2023-07-18  Lukas Klingsbo  docs: Fix grammar and typos in Checks readme (#2058)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/54a4c59..36d8617):
  36d8617  2023-07-18  dependabot[bot]  Bump actions/checkout from 3.5.2 to 3.5.3 (#53)

vector_math (https://github.com/google/vector_math.dart/compare/c147038..048777a):
  048777a  2023-07-19  moritzblume  Fix rotation around Y axis (#262)
  47a08ea  2023-07-17  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.2 (#293)

Change-Id: Id8010082c818c995aa7968327f98f347ff45761d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/315221
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

6 participants