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

Issue 2554123002: Support parallel captures from the StackSamplingProfiler. (Closed)

Created:
4 years ago by bcwhite
Modified:
3 years, 7 months ago
Reviewers:
Mike Wittman, gab, brettw
CC:
chromium-reviews, vmpstr+watch_chromium.org, Alexei Svitkine (slow)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Support parallel captures from the StackSamplingProfiler. Previously, only one sampling operation could be running and it was generally used to profile the startup of the browser. To make it more useful, it can now run against any thread and multiple profilers can execute in parallel. Sampling will continue until the desired number of samples has been collected, it is manually stopped, or the controlling object gets destructed. The SamplingThread is a singleton base::Thread that is self-managing. - It is started (via GetOrCreateTaskRunnerForAdd) on the calling thread when work arrives. - It stops (via ShutdownTask) on its own thread when it has been idle for 1 minute. - DetachFromSequence is called after both of these to allow for accessing the API from different threads. - thread_execution_state_lock_ is held when doing Thread API calls to ensure that access is sequenced. The sampled thread is expected to live at least as long as the thread controlling the sampling. SHERIFFS: Don't hesitate to roll this back if it correlates well with some kind of instability. Sampling has been known to have odd effects in the past and this rewrites a large part of it. BUG=671716 Review-Url: https://codereview.chromium.org/2554123002 Cr-Commit-Position: refs/heads/master@{#465614} Committed: https://chromium.googlesource.com/chromium/src/+/69e964496800e75cb0e3cdd974436659bd24e9cf

Patch Set 1 #

Total comments: 13

Patch Set 2 : move to Thread and MessageLoop #

Total comments: 31

Patch Set 3 : switched to task-runner and address review comments #

Total comments: 23

Patch Set 4 : rebased #

Patch Set 5 : addressed review comments by wittman #

Patch Set 6 : pass ID instead of pointers #

Total comments: 2

Patch Set 7 : addressed review comments by wittman #

Patch Set 8 : rebased #

Patch Set 9 : use helper methods for getting task runner #

Total comments: 8

Patch Set 10 : some minor cleanup #

Patch Set 11 : rebased #

Patch Set 12 : working shutdown, both idle and forced, with tests #

Patch Set 13 : support for death of thread-under-test #

Total comments: 22

Patch Set 14 : improved detection of thread state #

Patch Set 15 : use events to track thread lifetime in tests #

Patch Set 16 : add check that thread under test doesn't change #

Patch Set 17 : removed unnecessary thread-id check and fix test appropriately #

Patch Set 18 : added prevention of using profiler on non-windows platforms #

Patch Set 19 : merged synchronized-stop CL #

Total comments: 22

Patch Set 20 : addressed review comments by wittman #

Patch Set 21 : fixed typo #

Total comments: 8

Patch Set 22 : remove shutdown(); comment improvements #

Patch Set 23 : fix deadlock problem with GetTaskRunner(); fix layout of thread_restrictions.h #

Total comments: 46

Patch Set 24 : addressed review comments by wittman #

Total comments: 11

Patch Set 25 : addressed review comments by wittman #

Total comments: 6

Patch Set 26 : addressed review comments by wittman #

Patch Set 27 : addressed review comments by wittman #

Total comments: 20

Patch Set 28 : addressed review comments by wittman #

Total comments: 33

Patch Set 29 : switch to separate thread-state variable #

Total comments: 16

Patch Set 30 : addressed review comments by wittman #

Patch Set 31 : addressed review comments by wittman #

Total comments: 49

Patch Set 32 : rebased #

Patch Set 33 : addressed review comments by wittman #

Total comments: 26

Patch Set 34 : addressed review comments by wittman #

Total comments: 28

Patch Set 35 : addressed review comments by wittman #

Total comments: 4

Patch Set 36 : addressed review comments by wittman #

Total comments: 11

Patch Set 37 : more tests; improved tests #

Total comments: 61

Patch Set 38 : more test improvements #

Total comments: 5

Patch Set 39 : addressed review comments by wittman #

Patch Set 40 : fixed signed/unsigned comparison in test #

Total comments: 10

Patch Set 41 : addressed review comments by wittman #

Total comments: 25

Patch Set 42 : addressed review comments by wittman #

Total comments: 47

Patch Set 43 : addressed review comments by wittman #

Total comments: 2

Patch Set 44 : addressed review comments by gab #

Patch Set 45 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1391 lines, -344 lines) Patch
M base/profiler/stack_sampling_profiler.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 3 chunks +79 lines, -65 lines 0 comments Download
M base/profiler/stack_sampling_profiler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 5 chunks +631 lines, -168 lines 0 comments Download
M base/profiler/stack_sampling_profiler_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 20 chunks +679 lines, -111 lines 0 comments Download
M base/threading/thread_restrictions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 414 (252 generated)
Mike Wittman
Haven't reviewed the logic in great detail but this approach looks reasonable to me, with ...
4 years ago (2016-12-06 21:04:58 UTC) #2
Mike Wittman
On 2016/12/06 21:04:58, Mike Wittman wrote: > I believe you'll need to join the profiler ...
4 years ago (2016-12-06 21:10:01 UTC) #3
bcwhite
> The standard mechanism for inter-thread communication in > Chrome is via PostTask to a ...
4 years ago (2016-12-07 15:15:30 UTC) #4
Mike Wittman
On 2016/12/07 15:15:30, bcwhite wrote: > > The standard mechanism for inter-thread communication in > ...
4 years ago (2016-12-07 16:25:02 UTC) #5
Mike Wittman
https://codereview.chromium.org/2554123002/diff/1/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1/base/profiler/stack_sampling_profiler.cc#newcode223 base/profiler/stack_sampling_profiler.cc:223: static subtle::AtomicWord next_capture_id_; On 2016/12/07 16:25:02, Mike Wittman wrote: ...
4 years ago (2016-12-07 17:20:42 UTC) #6
bcwhite
> > > The standard mechanism for inter-thread communication in > > > Chrome is ...
4 years ago (2016-12-07 17:48:21 UTC) #7
Mike Wittman
On 2016/12/07 17:48:21, bcwhite wrote: > > > > The standard mechanism for inter-thread communication ...
4 years ago (2016-12-07 18:58:21 UTC) #8
bcwhite
On 2016/12/07 18:58:21, Mike Wittman wrote: > On 2016/12/07 17:48:21, bcwhite wrote: > > > ...
4 years ago (2016-12-07 19:54:24 UTC) #9
Mike Wittman
On 2016/12/07 19:54:24, bcwhite wrote: > On 2016/12/07 18:58:21, Mike Wittman wrote: > > On ...
4 years ago (2016-12-07 20:36:33 UTC) #10
bcwhite
New patch set using a message loop! Still rough but tests pass... except for the ...
4 years ago (2016-12-09 17:58:23 UTC) #11
Mike Wittman
On 2016/12/09 17:58:23, bcwhite wrote: > New patch set using a message loop! Great! Made ...
4 years ago (2016-12-09 21:45:02 UTC) #12
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); > According to the Thread ...
4 years ago (2016-12-09 23:38:31 UTC) #18
Mike Wittman
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/09 23:38:30, bcwhite wrote: ...
4 years ago (2016-12-10 00:24:23 UTC) #19
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/10 00:24:23, Mike Wittman ...
4 years ago (2016-12-13 16:08:11 UTC) #20
Mike Wittman
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/13 16:08:11, bcwhite wrote: ...
4 years ago (2016-12-13 18:16:41 UTC) #21
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); > It may be possible ...
4 years ago (2016-12-14 15:37:59 UTC) #22
Mike Wittman
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/14 15:37:59, bcwhite wrote: ...
4 years ago (2016-12-14 18:00:48 UTC) #23
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); > > Wasn't there an ...
4 years ago (2016-12-14 19:39:39 UTC) #24
Mike Wittman
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/14 19:39:39, bcwhite wrote: ...
4 years ago (2016-12-14 20:47:51 UTC) #25
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); > > I must be ...
4 years ago (2016-12-15 11:42:15 UTC) #26
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/15 11:42:15, bcwhite wrote: ...
4 years ago (2016-12-15 15:01:16 UTC) #27
Mike Wittman
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode311 base/profiler/stack_sampling_profiler.cc:311: scoped_refptr<SingleThreadTaskRunner> runner = task_runner(); On 2016/12/15 15:01:16, bcwhite wrote: ...
4 years ago (2016-12-15 17:22:46 UTC) #28
bcwhite
Switched to task-runner. Still a bit rough and tests need to be updated/added -- working ...
4 years ago (2016-12-15 18:07:50 UTC) #29
Mike Wittman
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode265 base/profiler/stack_sampling_profiler.cc:265: void PerformCapture(ActiveCapture* capture); On 2016/12/15 18:07:50, bcwhite wrote: > ...
4 years ago (2016-12-15 20:37:53 UTC) #30
bcwhite
https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/20001/base/profiler/stack_sampling_profiler.cc#newcode265 base/profiler/stack_sampling_profiler.cc:265: void PerformCapture(ActiveCapture* capture); On 2016/12/15 20:37:53, Mike Wittman wrote: ...
4 years ago (2016-12-21 16:39:11 UTC) #42
Mike Wittman
https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode285 base/profiler/stack_sampling_profiler.cc:285: std::map<int, WeakPtr<ActiveCapture>> active_captures_; On 2016/12/21 16:39:10, bcwhite wrote: > ...
4 years ago (2016-12-21 19:38:41 UTC) #47
bcwhite
https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode188 base/profiler/stack_sampling_profiler.cc:188: bool UpdateNextSampleTime() { > structs should not have methods ...
4 years ago (2016-12-22 16:12:10 UTC) #53
Mike Wittman
https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode188 base/profiler/stack_sampling_profiler.cc:188: bool UpdateNextSampleTime() { On 2016/12/22 16:12:10, bcwhite wrote: > ...
4 years ago (2016-12-22 17:38:22 UTC) #56
bcwhite
https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode188 base/profiler/stack_sampling_profiler.cc:188: bool UpdateNextSampleTime() { On 2016/12/22 17:38:22, Mike Wittman wrote: ...
3 years, 11 months ago (2017-01-05 16:35:58 UTC) #62
Mike Wittman
https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode400 base/profiler/stack_sampling_profiler.cc:400: bool success = task_runner()->PostDelayedTask( On 2017/01/05 16:35:58, bcwhite wrote: ...
3 years, 11 months ago (2017-01-05 21:08:39 UTC) #65
bcwhite
https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode400 base/profiler/stack_sampling_profiler.cc:400: bool success = task_runner()->PostDelayedTask( On 2017/01/05 21:08:39, Mike Wittman ...
3 years, 11 months ago (2017-01-05 22:04:22 UTC) #66
Mike Wittman
On 2017/01/05 22:04:22, bcwhite wrote: > https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc > File base/profiler/stack_sampling_profiler.cc (right): > > https://codereview.chromium.org/2554123002/diff/60001/base/profiler/stack_sampling_profiler.cc#newcode400 > ...
3 years, 11 months ago (2017-01-05 23:30:32 UTC) #67
bcwhite
> > > I think it would be better to create and use explicit functions ...
3 years, 11 months ago (2017-01-06 15:32:59 UTC) #74
Mike Wittman
On 2017/01/06 15:32:59, bcwhite wrote: > > > > I think it would be better ...
3 years, 11 months ago (2017-01-06 16:02:40 UTC) #75
bcwhite
https://codereview.chromium.org/2554123002/diff/240001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/240001/base/profiler/stack_sampling_profiler.cc#newcode232 base/profiler/stack_sampling_profiler.cc:232: // Get tas krunner that is usable from the ...
3 years, 11 months ago (2017-01-06 16:43:52 UTC) #78
Mike Wittman
Thanks, at a high level I think this is looking good at this point. The ...
3 years, 11 months ago (2017-01-06 17:45:51 UTC) #79
bcwhite
> 1. Comprehensive tests of the new functionality. I will be surprised if these > ...
3 years, 11 months ago (2017-01-06 20:47:57 UTC) #82
Mike Wittman
On 2017/01/06 20:47:57, bcwhite wrote: > > 1. Comprehensive tests of the new functionality. I ...
3 years, 11 months ago (2017-01-06 21:19:24 UTC) #83
Mike Wittman
Two other things remaining to do also come to mind: Using a single stack buffer ...
3 years, 11 months ago (2017-01-07 02:29:18 UTC) #84
bcwhite
> Using a single stack buffer for all captures. This needs to be implemented > ...
3 years, 11 months ago (2017-01-16 15:26:08 UTC) #94
bcwhite
> No, this was handled before before and is not handled in the current code. ...
3 years, 11 months ago (2017-01-16 16:09:52 UTC) #95
bcwhite
On 2017/01/16 16:09:52, bcwhite wrote: > > No, this was handled before before and is ...
3 years, 11 months ago (2017-01-16 20:08:19 UTC) #96
Mike Wittman
On 2017/01/16 20:08:19, bcwhite wrote: > On 2017/01/16 16:09:52, bcwhite wrote: > > > No, ...
3 years, 11 months ago (2017-01-17 17:01:21 UTC) #97
bcwhite
On 2017/01/17 17:01:21, Mike Wittman wrote: > On 2017/01/16 20:08:19, bcwhite wrote: > > On ...
3 years, 11 months ago (2017-01-17 17:07:41 UTC) #98
Mike Wittman
On 2017/01/17 17:07:41, bcwhite wrote: > On 2017/01/17 17:01:21, Mike Wittman wrote: > > On ...
3 years, 11 months ago (2017-01-17 17:19:33 UTC) #99
Mike Wittman
On 2017/01/16 15:26:08, bcwhite wrote: > > Using a single stack buffer for all captures. ...
3 years, 11 months ago (2017-01-17 17:37:40 UTC) #100
bcwhite
Finally got a working start/stop solution that supports browser shutdown and idle shutdown. Still need ...
3 years, 11 months ago (2017-01-25 22:02:17 UTC) #104
Mike Wittman
On 2017/01/25 22:02:17, bcwhite wrote: > Finally got a working start/stop solution that supports browser ...
3 years, 11 months ago (2017-01-26 02:25:01 UTC) #109
bcwhite
> A meta point: can we implement and review the support for the profiled thread ...
3 years, 10 months ago (2017-01-31 17:58:56 UTC) #121
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-01-31 22:14:27 UTC) #124
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 14:47:29 UTC) #127
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler_unittest.cc#newcode899 base/profiler/stack_sampling_profiler_unittest.cc:899: PlatformThread::Sleep(TimeDelta::FromSeconds(3)); On 2017/01/31 22:14:27, Mike Wittman wrote: > Coordinating ...
3 years, 10 months ago (2017-02-01 17:53:10 UTC) #132
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 17:59:54 UTC) #139
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 19:11:16 UTC) #141
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 20:26:51 UTC) #144
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 20:37:59 UTC) #145
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 21:19:04 UTC) #146
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-01 22:01:47 UTC) #147
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-02 02:48:05 UTC) #148
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-02 14:24:25 UTC) #149
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-02 19:22:31 UTC) #154
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-02 20:46:16 UTC) #155
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-02 22:29:15 UTC) #156
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-03 13:28:39 UTC) #157
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-03 17:24:15 UTC) #158
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-03 18:47:58 UTC) #161
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-04 01:09:29 UTC) #164
bcwhite
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-04 02:07:01 UTC) #165
Mike Wittman
https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/400001/base/profiler/stack_sampling_profiler.cc#newcode474 base/profiler/stack_sampling_profiler.cc:474: // An empty result indicates that the thread under ...
3 years, 10 months ago (2017-02-06 20:54:06 UTC) #166
Alexei Svitkine (slow)
Thanks for looping me in. I tried to read through a good chunk of recent ...
3 years, 10 months ago (2017-02-06 22:44:35 UTC) #168
bcwhite
> I tried to read through a good chunk of recent discussion here and hopefully ...
3 years, 10 months ago (2017-02-07 14:04:46 UTC) #169
Alexei Svitkine (slow)
Thanks for clarifying - sorry for not getting the full context the first time through. ...
3 years, 10 months ago (2017-02-07 15:58:26 UTC) #170
Mike Wittman
On 2017/02/07 15:58:26, Alexei Svitkine (slow) wrote: > Thanks for clarifying - sorry for not ...
3 years, 10 months ago (2017-02-07 18:23:24 UTC) #179
bcwhite
> > In this case I agree that we should go with the simpler solution ...
3 years, 10 months ago (2017-02-07 19:18:40 UTC) #180
Mike Wittman
We still haven't reached definitive conclusions on the known open questions I raised, which corroborates ...
3 years, 10 months ago (2017-02-07 21:44:28 UTC) #183
bcwhite
On 2017/02/07 21:44:28, Mike Wittman wrote: > We still haven't reached definitive conclusions on the ...
3 years, 10 months ago (2017-02-07 23:04:07 UTC) #184
Mike Wittman
On 2017/02/07 23:04:07, bcwhite wrote: > But the Windows native stack sampler never sets THREAD_EXITED ...
3 years, 10 months ago (2017-02-10 01:36:09 UTC) #185
bcwhite
> > But the Windows native stack sampler never sets THREAD_EXITED because > > it ...
3 years, 10 months ago (2017-02-10 14:47:18 UTC) #186
Mike Wittman
On 2017/02/10 14:47:18, bcwhite wrote: > > > But the Windows native stack sampler never ...
3 years, 10 months ago (2017-02-10 17:28:02 UTC) #187
bcwhite
> > > If this approach can't tell us when the thread has exited, then ...
3 years, 10 months ago (2017-02-10 18:36:06 UTC) #188
Mike Wittman
On 2017/02/10 18:36:06, bcwhite wrote: > > > > If this approach can't tell us ...
3 years, 10 months ago (2017-02-10 21:00:17 UTC) #189
bcwhite
> > > By "this approach" I mean the entire strategy of handling thread exit ...
3 years, 10 months ago (2017-02-11 02:57:10 UTC) #190
Mike Wittman
On 2017/02/11 02:57:10, bcwhite wrote: > > Treating them all as thread exit samples does ...
3 years, 10 months ago (2017-02-13 18:12:44 UTC) #191
bcwhite
> > > Treating them all as thread exit samples does not work because it ...
3 years, 10 months ago (2017-02-13 18:25:38 UTC) #192
Mike Wittman
On 2017/02/13 18:25:38, bcwhite wrote: > > > > Treating them all as thread exit ...
3 years, 10 months ago (2017-02-13 18:32:12 UTC) #193
bcwhite
> > > > > Treating them all as thread exit samples does not work ...
3 years, 10 months ago (2017-02-13 18:46:39 UTC) #194
Mike Wittman
On 2017/02/13 18:46:39, bcwhite wrote: > > > > > > Treating them all as ...
3 years, 10 months ago (2017-02-13 18:58:43 UTC) #195
bcwhite
Sync-stop CL merged into this one. PTAL
3 years, 10 months ago (2017-02-13 21:08:52 UTC) #198
Mike Wittman
On 2017/02/13 21:08:52, bcwhite wrote: > Sync-stop CL merged into this one. PTAL Thanks. https://codereview.chromium.org/2554123002/diff/580001/base/profiler/native_stack_sampler.h ...
3 years, 10 months ago (2017-02-13 22:35:57 UTC) #201
Mike Wittman
We'll probably need to special case the ThreadRestrictions::AssertWaitAllowed implementation for this to address the test ...
3 years, 10 months ago (2017-02-13 22:41:04 UTC) #202
Mike Wittman
On 2017/02/13 22:41:04, Mike Wittman wrote: > We'll probably need to special case the ThreadRestrictions::AssertWaitAllowed ...
3 years, 10 months ago (2017-02-13 22:43:21 UTC) #203
bcwhite
https://codereview.chromium.org/2554123002/diff/580001/base/profiler/native_stack_sampler.h File base/profiler/native_stack_sampler.h (right): https://codereview.chromium.org/2554123002/diff/580001/base/profiler/native_stack_sampler.h#newcode24 base/profiler/native_stack_sampler.h:24: // The thread state as determined by the sampler ...
3 years, 10 months ago (2017-02-14 14:33:02 UTC) #209
bcwhite
brettw@chromium.org: Please review changes in base/threading/thread_restrictions.h Rationale is provided here: https://codereview.chromium.org/2554123002/diff/620001/base/profiler/stack_sampling_profiler.cc line 614 // The ...
3 years, 10 months ago (2017-02-14 14:37:03 UTC) #211
Mike Wittman
On 2017/02/14 14:37:03, bcwhite wrote: > mailto:brettw@chromium.org: Please review changes in > base/threading/thread_restrictions.h > > ...
3 years, 10 months ago (2017-02-14 16:17:34 UTC) #216
bcwhite
On 2017/02/14 16:17:34, Mike Wittman wrote: > On 2017/02/14 14:37:03, bcwhite wrote: > > mailto:brettw@chromium.org: ...
3 years, 10 months ago (2017-02-14 17:32:44 UTC) #217
Mike Wittman
On 2017/02/14 17:32:44, bcwhite wrote: > On 2017/02/14 16:17:34, Mike Wittman wrote: > > On ...
3 years, 10 months ago (2017-02-14 17:50:56 UTC) #220
Mike Wittman
https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h File base/profiler/stack_sampling_profiler.h (right): https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h#newcode200 base/profiler/stack_sampling_profiler.h:200: // IMPORTANT: This should generally be created on the ...
3 years, 10 months ago (2017-02-14 17:52:32 UTC) #221
bcwhite
https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h File base/profiler/stack_sampling_profiler.h (right): https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h#newcode200 base/profiler/stack_sampling_profiler.h:200: // IMPORTANT: This should generally be created on the ...
3 years, 10 months ago (2017-02-14 20:13:27 UTC) #227
Mike Wittman
Still thinking through ShutdownTask() and the tests... https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h File base/profiler/stack_sampling_profiler.h (right): https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h#newcode241 base/profiler/stack_sampling_profiler.h:241: static void ...
3 years, 10 months ago (2017-02-15 03:26:44 UTC) #232
bcwhite
https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h File base/profiler/stack_sampling_profiler.h (right): https://codereview.chromium.org/2554123002/diff/640001/base/profiler/stack_sampling_profiler.h#newcode241 base/profiler/stack_sampling_profiler.h:241: static void Shutdown(); On 2017/02/15 03:26:44, Mike Wittman wrote: ...
3 years, 10 months ago (2017-02-15 16:17:35 UTC) #235
Mike Wittman
Wow, supporting restartable threads certainly makes for tons of complications around thread start and thread ...
3 years, 10 months ago (2017-02-15 21:56:01 UTC) #238
Mike Wittman
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode445 base/profiler/stack_sampling_profiler.cc:445: collection->next_sample_time = Time::Now(); On 2017/02/15 16:17:34, bcwhite wrote: > ...
3 years, 10 months ago (2017-02-15 22:52:16 UTC) #239
bcwhite
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode415 base/profiler/stack_sampling_profiler.cc:415: CollectionContext* collection = collection_ptr.get(); On 2017/02/15 21:56:00, Mike Wittman ...
3 years, 10 months ago (2017-02-16 17:39:49 UTC) #242
Mike Wittman
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode445 base/profiler/stack_sampling_profiler.cc:445: collection->next_sample_time = Time::Now(); On 2017/02/16 17:39:49, bcwhite wrote: > ...
3 years, 10 months ago (2017-02-17 16:10:19 UTC) #245
bcwhite
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode562 base/profiler/stack_sampling_profiler.cc:562: native_sampler_ = NativeStackSampler::Create(thread_id_, &RecordAnnotations, > I just remembered why ...
3 years, 10 months ago (2017-02-21 16:21:19 UTC) #248
Mike Wittman
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode562 base/profiler/stack_sampling_profiler.cc:562: native_sampler_ = NativeStackSampler::Create(thread_id_, &RecordAnnotations, On 2017/02/21 16:21:19, bcwhite wrote: ...
3 years, 10 months ago (2017-02-21 18:57:02 UTC) #249
bcwhite
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode562 base/profiler/stack_sampling_profiler.cc:562: native_sampler_ = NativeStackSampler::Create(thread_id_, &RecordAnnotations, > If use and destruction ...
3 years, 10 months ago (2017-02-21 21:48:05 UTC) #253
Mike Wittman
https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/700001/base/profiler/stack_sampling_profiler.cc#newcode562 base/profiler/stack_sampling_profiler.cc:562: native_sampler_ = NativeStackSampler::Create(thread_id_, &RecordAnnotations, On 2017/02/21 21:48:05, bcwhite wrote: ...
3 years, 10 months ago (2017-02-22 03:06:48 UTC) #257
bcwhite
https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc#newcode220 base/profiler/stack_sampling_profiler.cc:220: int task_runner_create_requests_ = 0; On 2017/02/22 03:06:48, Mike Wittman ...
3 years, 10 months ago (2017-02-22 14:32:51 UTC) #262
Mike Wittman
https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc#newcode504 base/profiler/stack_sampling_profiler.cc:504: task_runner_ = nullptr; On 2017/02/22 14:32:51, bcwhite wrote: > ...
3 years, 10 months ago (2017-02-22 20:32:18 UTC) #263
bcwhite
https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc#newcode504 base/profiler/stack_sampling_profiler.cc:504: task_runner_ = nullptr; > Just using it in place ...
3 years, 10 months ago (2017-02-22 20:51:15 UTC) #264
Mike Wittman
https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc#newcode504 base/profiler/stack_sampling_profiler.cc:504: task_runner_ = nullptr; On 2017/02/22 20:51:15, bcwhite wrote: > ...
3 years, 10 months ago (2017-02-22 21:11:13 UTC) #265
bcwhite
https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc#newcode504 base/profiler/stack_sampling_profiler.cc:504: task_runner_ = nullptr; On 2017/02/22 21:11:13, Mike Wittman wrote: ...
3 years, 10 months ago (2017-02-23 16:08:33 UTC) #268
Mike Wittman
https://codereview.chromium.org/2554123002/diff/820001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/820001/base/profiler/stack_sampling_profiler.cc#newcode187 base/profiler/stack_sampling_profiler.cc:187: enum ThreadExecutionState { This needs substantial comments explaining the ...
3 years, 10 months ago (2017-02-23 18:26:52 UTC) #273
bcwhite
https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/780001/base/profiler/stack_sampling_profiler.cc#newcode600 base/profiler/stack_sampling_profiler.cc:600: finished_event_.Wait(); On 2017/02/22 20:32:17, Mike Wittman wrote: > On ...
3 years, 10 months ago (2017-02-24 20:39:25 UTC) #285
Mike Wittman
https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc#newcode858 base/profiler/stack_sampling_profiler_unittest.cc:858: sampling_completed.TimedWait(AVeryLongTimeDelta()); On 2017/02/24 20:39:24, bcwhite wrote: > On 2017/02/22 ...
3 years, 9 months ago (2017-02-27 23:27:35 UTC) #286
bcwhite
https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc#newcode858 base/profiler/stack_sampling_profiler_unittest.cc:858: sampling_completed.TimedWait(AVeryLongTimeDelta()); On 2017/02/27 23:27:34, Mike Wittman wrote: > On ...
3 years, 9 months ago (2017-03-13 18:50:18 UTC) #293
Mike Wittman
https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc#newcode911 base/profiler/stack_sampling_profiler_unittest.cc:911: PlatformThread::YieldCurrentThread(); On 2017/03/13 18:50:17, bcwhite wrote: > On 2017/02/22 ...
3 years, 9 months ago (2017-03-14 18:57:34 UTC) #296
bcwhite
https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/800001/base/profiler/stack_sampling_profiler_unittest.cc#newcode911 base/profiler/stack_sampling_profiler_unittest.cc:911: PlatformThread::YieldCurrentThread(); On 2017/03/14 18:57:33, Mike Wittman wrote: > On ...
3 years, 9 months ago (2017-03-16 15:56:25 UTC) #299
Mike Wittman
Still taking another look at the ConcurrentProfiling_* tests. https://codereview.chromium.org/2554123002/diff/880001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/880001/base/profiler/stack_sampling_profiler_unittest.cc#newcode931 base/profiler/stack_sampling_profiler_unittest.cc:931: params[0].samples_per_burst ...
3 years, 9 months ago (2017-03-18 01:38:41 UTC) #302
Mike Wittman
https://codereview.chromium.org/2554123002/diff/940001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/940001/base/profiler/stack_sampling_profiler.cc#newcode543 base/profiler/stack_sampling_profiler.cc:543: return; On 2017/03/18 01:38:41, Mike Wittman wrote: > I ...
3 years, 9 months ago (2017-03-20 14:59:40 UTC) #303
bcwhite
https://codereview.chromium.org/2554123002/diff/880001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/880001/base/profiler/stack_sampling_profiler_unittest.cc#newcode931 base/profiler/stack_sampling_profiler_unittest.cc:931: params[0].samples_per_burst = 10; On 2017/03/18 01:38:41, Mike Wittman wrote: ...
3 years, 9 months ago (2017-03-20 21:50:51 UTC) #306
Mike Wittman
https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler.cc#newcode242 base/profiler/stack_sampling_profiler.cc:242: Lock task_runner_lock_; On 2017/03/20 21:50:51, bcwhite wrote: > On ...
3 years, 9 months ago (2017-03-21 16:50:38 UTC) #309
bcwhite
https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler.cc#newcode242 base/profiler/stack_sampling_profiler.cc:242: Lock task_runner_lock_; On 2017/03/21 16:50:38, Mike Wittman wrote: > ...
3 years, 9 months ago (2017-03-22 17:48:55 UTC) #312
Mike Wittman
https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc#newcode940 base/profiler/stack_sampling_profiler_unittest.cc:940: TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_InSync) { > > > > > Remove: ...
3 years, 9 months ago (2017-03-23 22:18:32 UTC) #315
bcwhite
more tests; improved tests
3 years, 9 months ago (2017-03-27 17:45:12 UTC) #316
bcwhite
Some re-work was necessary to fix tests when --gtest_shuffle is set. https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): ...
3 years, 9 months ago (2017-03-27 17:52:43 UTC) #319
Mike Wittman
Seems like we're converging on the tests covering the lifetime behaviors. Can you also provide ...
3 years, 8 months ago (2017-03-28 19:32:14 UTC) #322
bcwhite
https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc#newcode940 base/profiler/stack_sampling_profiler_unittest.cc:940: TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_InSync) { On 2017/03/28 19:32:01, Mike Wittman wrote: ...
3 years, 8 months ago (2017-03-29 14:56:59 UTC) #325
Mike Wittman
https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc#newcode940 base/profiler/stack_sampling_profiler_unittest.cc:940: TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_InSync) { On 2017/03/29 14:56:57, bcwhite wrote: > ...
3 years, 8 months ago (2017-03-30 16:18:39 UTC) #328
bcwhite
https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/920001/base/profiler/stack_sampling_profiler_unittest.cc#newcode940 base/profiler/stack_sampling_profiler_unittest.cc:940: TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling_InSync) { On 2017/03/30 16:18:38, Mike Wittman wrote: ...
3 years, 8 months ago (2017-03-30 18:54:51 UTC) #331
Mike Wittman
With the changes below this should be looking pretty good. I'm going to make another ...
3 years, 8 months ago (2017-03-31 01:38:22 UTC) #342
Mike Wittman
Hi gab, can you review the Thread API usage in this change for consistency with ...
3 years, 8 months ago (2017-03-31 01:46:33 UTC) #344
bcwhite
https://codereview.chromium.org/2554123002/diff/1000001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1000001/base/profiler/stack_sampling_profiler.cc#newcode288 base/profiler/stack_sampling_profiler.cc:288: DCHECK(sampler->active_collections_.empty()); On 2017/03/31 01:38:21, Mike Wittman wrote: > On ...
3 years, 8 months ago (2017-03-31 13:57:56 UTC) #351
bcwhite
@brettw: stack_sampling_profiler.cc, 738 // The behavior of sampling a thread that has exited is undefined ...
3 years, 8 months ago (2017-03-31 13:59:19 UTC) #352
brettw
Can you write a better CL description? I don't really know what's going on.
3 years, 8 months ago (2017-03-31 17:12:44 UTC) #355
Mike Wittman
https://codereview.chromium.org/2554123002/diff/1020001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/1020001/base/profiler/stack_sampling_profiler_unittest.cc#newcode1611 base/profiler/stack_sampling_profiler_unittest.cc:1611: ProfilerThread profiler2("profiler2", target_thread_id, params2); On 2017/03/31 13:57:56, bcwhite wrote: ...
3 years, 8 months ago (2017-03-31 18:12:33 UTC) #356
gab
On 2017/03/31 01:46:33, Mike Wittman wrote: > Hi gab, can you review the Thread API ...
3 years, 8 months ago (2017-04-03 17:00:19 UTC) #359
bcwhite
CL description changes: Done. https://codereview.chromium.org/2554123002/diff/1080001/base/profiler/stack_sampling_profiler_unittest.cc File base/profiler/stack_sampling_profiler_unittest.cc (right): https://codereview.chromium.org/2554123002/diff/1080001/base/profiler/stack_sampling_profiler_unittest.cc#newcode637 base/profiler/stack_sampling_profiler_unittest.cc:637: StackSamplingProfiler::ResetAnnotationsForTesting(); > Given these facts, ...
3 years, 8 months ago (2017-04-03 20:18:13 UTC) #368
brettw
owners lgtm but I mostly only looked at the API. Be sure to follow up ...
3 years, 8 months ago (2017-04-03 21:42:51 UTC) #369
bcwhite
https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc#newcode291 base/profiler/stack_sampling_profiler.cc:291: CHECK(sampler->active_collections_.empty()); On 2017/04/03 21:42:51, brettw (plz ping after 24h) ...
3 years, 8 months ago (2017-04-04 12:59:31 UTC) #370
Alexei Svitkine (slow)
https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc#newcode291 base/profiler/stack_sampling_profiler.cc:291: CHECK(sampler->active_collections_.empty()); On 2017/04/04 12:59:31, bcwhite wrote: > On 2017/04/03 ...
3 years, 8 months ago (2017-04-04 15:44:26 UTC) #371
bcwhite
https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc#newcode291 base/profiler/stack_sampling_profiler.cc:291: CHECK(sampler->active_collections_.empty()); On 2017/04/04 15:44:26, Alexei Svitkine (slow) wrote: > ...
3 years, 8 months ago (2017-04-04 15:54:49 UTC) #372
Mike Wittman
This is looking good to me. Just waiting for gab's review. A couple comments on ...
3 years, 8 months ago (2017-04-04 17:59:52 UTC) #373
brettw
The DCHECK/CHECK thing doesn't matter either way in practice for this patch so I want ...
3 years, 8 months ago (2017-04-04 18:20:55 UTC) #374
gab
Some comments, some drive-bys on a few nits I spotted and a meta-comment: Why does ...
3 years, 8 months ago (2017-04-05 20:38:43 UTC) #380
bcwhite
https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc#newcode291 base/profiler/stack_sampling_profiler.cc:291: CHECK(sampler->active_collections_.empty()); On 2017/04/04 17:59:51, Mike Wittman wrote: > On ...
3 years, 8 months ago (2017-04-06 16:18:51 UTC) #381
Mike Wittman
https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc#newcode291 base/profiler/stack_sampling_profiler.cc:291: CHECK(sampler->active_collections_.empty()); On 2017/04/06 16:18:49, bcwhite wrote: > On 2017/04/04 ...
3 years, 8 months ago (2017-04-06 16:55:41 UTC) #382
bcwhite
https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc File base/profiler/stack_sampling_profiler.cc (right): https://codereview.chromium.org/2554123002/diff/1150001/base/profiler/stack_sampling_profiler.cc#newcode114 base/profiler/stack_sampling_profiler.cc:114: class StackSamplingProfiler::SamplingThread : public Thread { On 2017/04/05 20:38:42, ...
3 years, 8 months ago (2017-04-06 18:40:19 UTC) #385
bcwhite
> Why does sampling have to be startable from any thread? Besides starting and > ...
3 years, 8 months ago (2017-04-06 18:44:03 UTC) #387
gab
On 2017/04/06 18:44:03, bcwhite wrote: > > Why does sampling have to be startable from ...
3 years, 8 months ago (2017-04-06 19:31:28 UTC) #388
Mike Wittman
LGTM Since this is a pretty major change, we should land it after next week's ...
3 years, 8 months ago (2017-04-06 21:54:30 UTC) #391
bcwhite
> Since this is a pretty major change, we should land it after next week's ...
3 years, 8 months ago (2017-04-07 15:41:55 UTC) #392
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2554123002/1190001
3 years, 8 months ago (2017-04-19 11:46:28 UTC) #395
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/414999) ios-device on master.tryserver.chromium.mac (JOB_FAILED, ...
3 years, 8 months ago (2017-04-19 11:49:49 UTC) #397
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2554123002/1210001
3 years, 8 months ago (2017-04-19 15:26:13 UTC) #404
commit-bot: I haz the power
Committed patchset #45 (id:1210001) as https://chromium.googlesource.com/chromium/src/+/69e964496800e75cb0e3cdd974436659bd24e9cf
3 years, 8 months ago (2017-04-19 15:30:50 UTC) #407
lijeffrey
On 2017/04/19 15:30:50, commit-bot: I haz the power wrote: > Committed patchset #45 (id:1210001) as ...
3 years, 8 months ago (2017-04-26 23:52:53 UTC) #410
lijeffrey
On 2017/04/26 23:52:53, lijeffrey wrote: > On 2017/04/19 15:30:50, commit-bot: I haz the power wrote: ...
3 years, 8 months ago (2017-04-26 23:54:12 UTC) #411
Mike Wittman
Can you provide a link to a build where this test fails? It's passing in ...
3 years, 8 months ago (2017-04-27 00:34:54 UTC) #412
lijeffrey
On 2017/04/27 00:34:54, Mike Wittman wrote: > Can you provide a link to a build ...
3 years, 7 months ago (2017-04-27 11:16:17 UTC) #413
Mike Wittman
3 years, 7 months ago (2017-04-27 16:51:38 UTC) #414
Message was sent while issue was closed.
This is a false positive. The failure appears to be due to some destruction
ordering error in Mac UI, which is entirely unrelated to the CL at hand.

On Thu, Apr 27, 2017 at 4:16 AM, <lijeffrey@chromium.org> wrote:

> On 2017/04/27 00:34:54, Mike Wittman wrote:
> > Can you provide a link to a build where this test fails? It's passing in
> > build 39545 linked in the analysis.
> >
> > In any case, this change is unlikely to be the cause of flakiness on Mac
> > because the modified functionality is only enabled for 64-bit Windows.
> >
> > On Wed, Apr 26, 2017 at 4:54 PM, <mailto:lijeffrey@chromium.org> wrote:
> >
> > > On 2017/04/26 23:52:53, lijeffrey wrote:
> > > > On 2017/04/19 15:30:50, commit-bot: I haz the power wrote:
> > > > > Committed patchset #45 (id:1210001) as
> > > > >
> > > >
> > > https://chromium.googlesource.com/chromium/src/+/
> > > 69e964496800e75cb0e3cdd974436659bd24e9cf
> > > >
> > > > Hey guys, Findit's analysis for a flaky test
> > > >
> > > "BrowserCloseManagerBrowserTest/BrowserCloseManagerBrowserTest
> > > .AddBeforeUnloadDuringClosing/0"
> > > > suggests this as the culprit according to analysis
> > > >
> > > ag9zfmZpbmRpdC1mb3ItbWVy6AELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9v
> > > dCKxAWNocm9taXVtLm1hYy9NYWMxMC45IFRlc3RzIChkYmcpLzM5NTUwL2Jy
> > > b3dzZXJfdGVzdHMvUW5KdmQzTmxja05zYjNObFRXRnVZV2RsY2tKeWIzZHpa
> > > WEpVWlhOMEwwSnliM2R6WlhKRGJHOXpaVTFoYm1GblpYSkNjbTkzYzJWeVZH
> > > VnpkQzVCWkdSQ1pXWnZjbVZWYm14dllXUkVkWEpwYm1kRGJHOXphVzVuTHpB
> > > PQwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM,
> > > > can someone please help verify?
> > > >
> > > > Thanks,
> > > > Jeff on behalf of Findit team
> > >
> > > Oops sorry here's the full link to the analysis:
> > >
> > > https://findit-for-me.appspot.com/waterfall/flake?key=
> > > ag9zfmZpbmRpdC1mb3ItbWVy6AELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9v
> > > dCKxAWNocm9taXVtLm1hYy9NYWMxMC45IFRlc3RzIChkYmcpLzM5NTUwL2Jy
> > > b3dzZXJfdGVzdHMvUW5KdmQzTmxja05zYjNObFRXRnVZV2RsY2tKeWIzZHpa
> > > WEpVWlhOMEwwSnliM2R6WlhKRGJHOXpaVTFoYm1GblpYSkNjbTkzYzJWeVZH
> > > VnpkQzVCWkdSQ1pXWnZjbVZWYm14dllXUkVkWEpwYm1kRGJHOXphVzVuTHpB
> > > PQwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM
> > >
> > > https://codereview.chromium.org/2554123002/
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
> Thanks for the reply!
> https://luci-milo.appspot.com/buildbot/chromium.mac/Mac10.9%
> 20Tests%20%28dbg%29/39548
> and
> https://luci-milo.appspot.com/buildbot/chromium.mac/Mac10.9%
> 20Tests%20%28dbg%29/39550
> both fail for the same test which appears to have started flaking after
> this CL
> landed. If it's a false positive please let us know so we can improve the
> flake
> analyzer! :)
>
> https://codereview.chromium.org/2554123002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698