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

performance: Stopwatch takes its time #17274

Closed
DartBot opened this issue Mar 5, 2014 · 7 comments
Closed

performance: Stopwatch takes its time #17274

DartBot opened this issue Mar 5, 2014 · 7 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant library-core type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Mar 5, 2014

This issue was originally filed by @tatumizer


Stopwatch, widely used to benchmark performance, itself performs very badly! One invocation of elapsedMicroseconds takes 0.3 microseconds in 32-bit mode! Which makes it nearly impossible to implement any performance tool based on stopwatch - you will be benchmarking Stopwatch instead of a program.

The culprit: external method _now that returns huge number that doesn't fit in smi.

There's very easy fix: store start time as double and return _now as double. 52 integer bits in double is more than enough. This will do wonders in terms of speed. On 64-bit processors, performance will be more or less the same as with ints.

@DartBot
Copy link
Author

DartBot commented Mar 5, 2014

This comment was originally written by @tatumizer


I, of course, meant only internal changes inside Stopwatch implementation, not API changes.

@lrhn
Copy link
Member

lrhn commented Mar 6, 2014

This is a VM only problem (since dart2js obviously uses doubles already).

Is this still a problem when your code gets optimized, or is it just the first initial calls to stopwatch that are slow?
(Pro-top: Always run your benchmarks until the code is warm before doing the real measurement - unless what you are measuring is start-up time).


Removed Type-Defect label.
Added Type-Enhancement, Area-Library, Library-Core, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Mar 6, 2014

This comment was originally written by @tatumizer


This is part of a set of benchmarks where I always use 3 warmups, but results show only first warmup makes sense. Please run on WIndows (32 bit). WIth 64-bit, this won't be a problem.
 
var N=10000000;
test4() {
  int x=0;
  var w=new Stopwatch()..start();
  for (int i=0; i<N; i++) {
    var n=w.elapsedMilliseconds;
    x|=(n&0xFF);
  }
  return x;
}
main() {
  test4(); // warmup
  w = new Stopwatch()..start();
  print("${test4()} ${w.elapsedMicroseconds}");

}

@floitschG
Copy link
Contributor

cc @fsc8000.

@DartBot
Copy link
Author

DartBot commented Mar 6, 2014

This comment was originally written by @tatumizer


More accurate data: 2.7 GHz processor i7, with all SIMD instructions imaginable:
elapsedMicroseconds - 285 nanoseconds per invocation of getter
elapsedTicks - 82 nanoseconds per invocation of getter

Neither is good.

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core labels Mar 6, 2014
@floitschG
Copy link
Contributor

Fwiw, I just ran this on my machine, and if I read the data correctly, it's now much faster.
Running the program on my machine takes 556413 (desktop) or 605918 (notebook) microseconds, which means that the elapsedMilliseconds getter took ~60 nanoseconds.

This was tested on my desktop (Intel(R) Xeon(R) CPU E5-2690 0 @ 2.90GHz) and my notebook (Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz).

elapsedTicks is ~10% faster.

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed priority-unassigned labels Feb 29, 2016
@lrhn lrhn added the core-m label Aug 11, 2017
@floitschG floitschG added closed-obsolete Closed as the reported issue is no longer relevant and removed core-m labels Aug 14, 2017
@floitschG
Copy link
Contributor

From my last measurements it looks like elapsedMilliseconds has acceptable performance now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants