Issue 324843004: Significantly improve performance of code coverage tool by precomputing a map token_pos->line numbe…

Issue 324843004: Significantly improve performance of code coverage tool by precomputing a map token_pos->line numbe… (Closed)

Created:
6 years, 6 months ago by srdjan

Modified:
6 years, 6 months ago

Reviewers:
siva, Ivan Posva

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Significantly improve performance of code coverage tool by precomputing a map token_pos->line number. R=asiva@google.com, iposva@google.com Committed: https://code.google.com/p/dart/source/detail?r=37203

Patch Set 6 : #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+42 lines, -9 lines)			Patch
	M	runtime/vm/coverage.h	View	1 2 3	2 chunks	+3 lines, -1 line	0 comments	Download
	M	runtime/vm/coverage.cc	View	1 2 3 4 5	8 chunks	+39 lines, -8 lines	0 comments	Download

Messages

Total messages: 9 (0 generated)

Expand Messages | Collapse Messages

siva

There are cases where a token post to line/column information. This speed up does not ...

6 years, 6 months ago (2014-06-09 23:28:08 UTC) #2

srdjan

On 2014/06/09 23:28:08, siva wrote: > There are cases where a token post to line/column ...

6 years, 6 months ago (2014-06-09 23:46:58 UTC) #3

Ivan Posva

https://codereview.chromium.org/324843004/diff/40001/runtime/vm/coverage.cc File runtime/vm/coverage.cc (right): https://codereview.chromium.org/324843004/diff/40001/runtime/vm/coverage.cc#newcode92 runtime/vm/coverage.cc:92: script.ComputeTokenPosToLineNumberArray(); As discussed we might want to cache this ...

6 years, 6 months ago (2014-06-10 00:14:59 UTC) #4

srdjan

Simplified code, made map token-pos -> line-number local to coverage.cc

6 years, 6 months ago (2014-06-10 20:36:46 UTC) #5

siva

lgtm https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc File runtime/vm/coverage.cc (right): https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc#newcode29 runtime/vm/coverage.cc:29: } I am wondering if we should have ...

6 years, 6 months ago (2014-06-10 21:31:55 UTC) #6

Ivan Posva

LGTMwC -Ivan https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc File runtime/vm/coverage.cc (right): https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc#newcode26 runtime/vm/coverage.cc:26: map->Clear(); map->ClearTo(-1); https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc#newcode117 runtime/vm/coverage.cc:117: if (pos_to_line.is_empty()) { ...

6 years, 6 months ago (2014-06-10 21:34:48 UTC) #7

srdjan

Committed patchset #6 manually as r37203 (presubmit successful).

6 years, 6 months ago (2014-06-10 21:37:55 UTC) #8

srdjan

6 years, 6 months ago (2014-06-10 21:38:01 UTC) #9

Message was sent while issue was closed.

https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc
File runtime/vm/coverage.cc (right):

https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc#n...
runtime/vm/coverage.cc:29: }
On 2014/06/10 21:31:55, siva wrote:
> I am wondering if we should have an AddAt or EnsureCapacity method in
> BaseGrowableArray, then we won't need this loop.

Agreed. Will do in a follow-up CL.

https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc#n...
runtime/vm/coverage.cc:117: if (pos_to_line.is_empty()) {
On 2014/06/10 21:34:48, Ivan Posva wrote:
> always?

Done.

https://codereview.chromium.org/324843004/diff/80001/runtime/vm/coverage.cc#n...
runtime/vm/coverage.cc:156: if (pos_to_line.is_empty()) {
On 2014/06/10 21:34:48, Ivan Posva wrote:
> ditto

Done.

Expand Messages | Collapse Messages