Speedup function profile by using a hash table instead of a linear scan

Speedup function profile by using a hash table instead of a linear scan

BUG=
R=iposva@google.com

Committed: https://github.com/dart-lang/sdk/commit/e389ab73d39e143fc90bbea834662e9229971301

[Cutch, 2016-03-23 01:15:30]

Description was changed from

==========
Speedup function profile by using a hash table instead of a linear scan

BUG=
==========

to

==========
Speedup function profile by using a hash table instead of a linear scan

BUG=
==========

[Cutch, 2016-03-23 01:15:30]

johnmccutchan@google.com changed reviewers:
+ asiva@google.com, iposva@google.com

[Ivan Posva, 2016-03-23 15:14:40]

LGTM with comments addressed.

-Ivan

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:22: DEFINE_FLAG(bool, trace_profiler, false,
"Trace profiler.");
Please move these flags into the common flags file and set them appropriately.
For example verbose only in debug builds.

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:605: return kv->key();
Personally I find these typedefs confusing, as well as the addition of the key()
accessor to ProfileFunction. Can we rename from key() to function()?

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.h (right):

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.h:82: }
Looking at this again this definitely should be consolidated to this being the
function() accessor.

[Cutch, 2016-03-23 19:02:42]

This change takes profile fetch time in Observatory for sra's reproduction from
42 seconds to 13. Still look for other low hanging fruit.

[Cutch, 2016-03-23 19:34:22]

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:22: DEFINE_FLAG(bool, trace_profiler, false,
"Trace profiler.");
On 2016/03/23 15:14:39, Ivan Posva wrote:
> Please move these flags into the common flags file and set them appropriately.
> For example verbose only in debug builds.

Done.

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:605: return kv->key();
On 2016/03/23 15:14:39, Ivan Posva wrote:
> Personally I find these typedefs confusing, as well as the addition of the
key()
> accessor to ProfileFunction. Can we rename from key() to function()?

I've renamed key() to function() but the typedefs are required by
DirectChainedHashMap.

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.h (right):

https://codereview.chromium.org/1830473002/diff/20001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.h:82: }
On 2016/03/23 15:14:40, Ivan Posva wrote:
> Looking at this again this definitely should be consolidated to this being the
> function() accessor.

Done.

[Cutch, 2016-03-23 19:36:01]

Description was changed from

==========
Speedup function profile by using a hash table instead of a linear scan

BUG=
==========

to

==========
Speedup function profile by using a hash table instead of a linear scan

BUG=
R=iposva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/e389ab73d39e143fc90bbea834662e9229971301
==========

[Cutch, 2016-03-23 19:36:03]

Committed patchset #3 (id:40001) manually as
e389ab73d39e143fc90bbea834662e9229971301 (presubmit successful).

[Ivan Posva, 2016-03-24 22:21:29]

Thanks!

Small observation about handle allocation...

-Ivan

https://codereview.chromium.org/1830473002/diff/40001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1830473002/diff/40001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2403: const Function& function =
Function::Handle(func->function()->raw());
Why do you need to allocate a new Function::Handle here?