[profiler] Allow thread-safe access to add/remove code event observers.

Allow thread-safe access to add/remove code event observers.

BUG=406277
Committed: https://crrev.com/1e937f6676ea5d714a56d59451d808ee41f6a98d
Cr-Commit-Position: refs/heads/master@{#39853}

[alph, 2016-09-09 09:44:40]

The CQ bit was checked by alph@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 09:44:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[alph, 2016-09-09 09:45:07]

alph@chromium.org changed reviewers:
+ bmeurer@chromium.org, jochen@chromium.org

[alph, 2016-09-09 09:45:08]

ptal

[commit-bot: I haz the power, 2016-09-09 10:13:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 10:13:01]

Dry run: This issue passed the CQ dry run.

[alph, 2016-09-09 10:18:16]

Description was changed from

==========
Allow thread-safe access to add/remove code event observers.

BUG=406277
==========

to

==========
Allow thread-safe access to add/remove code event observers.

BUG=406277
==========

[jochen (gone - plz use gerrit), 2016-09-09 13:21:09]

https://codereview.chromium.org/2321073004/diff/1/src/profiler/profiler-liste...
File src/profiler/profiler-listener.h (right):

https://codereview.chromium.org/2321073004/diff/1/src/profiler/profiler-liste...
src/profiler/profiler-listener.h:83: base::LockGuard<base::Mutex>
guard(&mutex_);
shouldn't you lock before checking observers_?

[alph, 2016-09-09 21:03:30]

https://codereview.chromium.org/2321073004/diff/1/src/profiler/profiler-liste...
File src/profiler/profiler-listener.h (right):

https://codereview.chromium.org/2321073004/diff/1/src/profiler/profiler-liste...
src/profiler/profiler-listener.h:83: base::LockGuard<base::Mutex>
guard(&mutex_);
On 2016/09/09 13:21:09, jochen wrote:
> shouldn't you lock before checking observers_?

I'd like to have a fast path if there're no observers (the common case).

It's ok to have a race here if another thread is adding/removing an observer at
this point.

According to http://www.cplusplus.com/reference/vector/vector/empty/
"No contained elements are accessed: concurrently accessing or modifying them is
safe."

[jochen (gone - plz use gerrit), 2016-09-12 10:01:42]

that just says that modifying elements is safe, however, in AddObserver you also
modify the container.

I also don't think it's slow to take the mutex since most likely nobody holds
it?

[alph, 2016-09-12 18:16:58]

On 2016/09/12 10:01:42, jochen wrote:
> that just says that modifying elements is safe, however, in AddObserver you
also
> modify the container.
> 
> I also don't think it's slow to take the mutex since most likely nobody holds
> it?

Ok, my assumption was that I can read the empty() value from the container
without the synchronization, because it will return either the old value (that
was here before other thread has modified it), or the new value. And both are
fine to get.

Anyway I removed the checks. ptal.

[alph, 2016-09-12 18:16:58]

On 2016/09/12 10:01:42, jochen wrote:
> that just says that modifying elements is safe, however, in AddObserver you
also
> modify the container.
> 
> I also don't think it's slow to take the mutex since most likely nobody holds
> it?

Ok, my assumption was that I can read the empty() value from the container
without the synchronization, because it will return either the old value (that
was here before other thread has modified it), or the new value. And both are
fine to get.

Anyway I removed the checks. ptal.

[jochen (gone - plz use gerrit), 2016-09-14 08:10:17]

lgtm

[alph, 2016-09-29 07:54:09]

The CQ bit was checked by alph@chromium.org

[commit-bot: I haz the power, 2016-09-29 07:54:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 08:24:04]

Description was changed from

==========
Allow thread-safe access to add/remove code event observers.

BUG=406277
==========

to

==========
Allow thread-safe access to add/remove code event observers.

BUG=406277
==========

[commit-bot: I haz the power, 2016-09-29 08:24:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-29 08:24:14]

Description was changed from

==========
Allow thread-safe access to add/remove code event observers.

BUG=406277
==========

to

==========
Allow thread-safe access to add/remove code event observers.

BUG=406277

Committed: https://crrev.com/1e937f6676ea5d714a56d59451d808ee41f6a98d
Cr-Commit-Position: refs/heads/master@{#39853}
==========

[commit-bot: I haz the power, 2016-09-29 08:24:15]

Patchset 2 (id:??) landed as
https://crrev.com/1e937f6676ea5d714a56d59451d808ee41f6a98d
Cr-Commit-Position: refs/heads/master@{#39853}