Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(77)

Issue 5633009: Collect only optimizable function samples.... (Closed)

Created:
10 years ago by Karl Klose
Modified:
9 years, 7 months ago
Reviewers:
fschneider, Kasper Lund
CC:
v8-dev
Visibility:
Public.

Description

Collect only optimizable function samples. Keep track of the ratio between JS and non-JS ticks and use this ratio to adjust the lookup threshold. (Also add support to trace compilation statistics.) Committed: http://code.google.com/p/v8/source/detail?r=5955

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 17

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Total comments: 10

Patch Set 8 : '' #

Patch Set 9 : '' #

Patch Set 10 : '' #

Patch Set 11 : '' #

Patch Set 12 : '' #

Total comments: 3

Patch Set 13 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -20 lines) Patch
M src/compiler.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +19 lines, -6 lines 0 comments Download
M src/flag-definitions.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime-profiler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 6 chunks +77 lines, -14 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Karl Klose
10 years ago (2010-12-08 12:56:36 UTC) #1
Karl Klose
http://golem.spb:9013/v8/r5946-v8-karlklose-jsratio.html#g
10 years ago (2010-12-08 12:57:28 UTC) #2
Karl Klose
New version that fixes the Kraken regression.
10 years ago (2010-12-08 14:00:49 UTC) #3
fschneider
drive-by: http://codereview.chromium.org/5633009/diff/14001/src/runtime-profiler.cc File src/runtime-profiler.cc (right): http://codereview.chromium.org/5633009/diff/14001/src/runtime-profiler.cc#newcode305 src/runtime-profiler.cc:305: if (js_ratio < .2) { These constants need ...
10 years ago (2010-12-08 14:33:32 UTC) #4
Kasper Lund
This is getting there. It's somewhat confusing that we have two concepts of samples: One ...
10 years ago (2010-12-08 14:54:15 UTC) #5
karlklose
> http://codereview.chromium.org/5633009/diff/14001/src/compiler.cc#newcode116 > src/compiler.cc:116: static double compilation_time_; > Convert these to statics inside the if ...
10 years ago (2010-12-09 10:09:45 UTC) #6
Kasper Lund
Much better. Comments: http://codereview.chromium.org/5633009/diff/23002/src/compiler.cc File src/compiler.cc (right): http://codereview.chromium.org/5633009/diff/23002/src/compiler.cc#newcode127 src/compiler.cc:127: if (FLAG_trace_opt) { Remove this check. ...
10 years ago (2010-12-09 11:51:51 UTC) #7
Karl Klose
http://codereview.chromium.org/5633009/diff/23002/src/compiler.cc File src/compiler.cc (right): http://codereview.chromium.org/5633009/diff/23002/src/compiler.cc#newcode127 src/compiler.cc:127: if (FLAG_trace_opt) { On 2010/12/09 11:51:51, Kasper Lund wrote: ...
10 years ago (2010-12-09 12:49:32 UTC) #8
Kasper Lund
10 years ago (2010-12-09 13:02:37 UTC) #9
LGTM!

http://codereview.chromium.org/5633009/diff/46001/src/runtime-profiler.cc
File src/runtime-profiler.cc (right):

http://codereview.chromium.org/5633009/diff/46001/src/runtime-profiler.cc#new...
src/runtime-profiler.cc:286: if (sampler_ticks_until_threshold_adjustment == 0)
{
Maybe make this <= 0 to be defensive.

http://codereview.chromium.org/5633009/diff/46001/src/runtime-profiler.cc#new...
src/runtime-profiler.cc:306: if (!IsOptimizable(function)) {
I would put this on one line.

http://codereview.chromium.org/5633009/diff/46001/src/runtime-profiler.cc#new...
src/runtime-profiler.cc:317: 
Remove this newline and add one below the line defining current_js_ratio.

Powered by Google App Engine
This is Rietveld 408576698