NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data.

It does not need synchronization or ordering constraints because it's
ok for child processes to observe a stale value because it's only used
to avoid sending a no-op IPC to browser process if child knows trial is
activated. Still, this came up during design review and it doesn't hurt
to use these.

BUG=673042
Committed: https://crrev.com/8095c757053bd2bc15b5b2570362587ba38c5779
Cr-Commit-Position: refs/heads/master@{#438235}

[Alexei Svitkine (slow), 2016-12-12 18:10:05]

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

[commit-bot: I haz the power, 2016-12-12 18:10:40]

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

[Alexei Svitkine (slow), 2016-12-12 18:11:45]

Patchset #1 (id:1) has been deleted

[Alexei Svitkine (slow), 2016-12-12 18:13:28]

Description was changed from

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data and it's ok for child processes to observe a stale value because
it's only used to avoid sending a no-op IPC to browser process if child
knows trial is activated. Still, this came up during design review and
it doesn't hurt to use these.

BUG=673042
==========

to

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data and it's ok for child processes to observe a stale value because
it's only used to avoid sending a no-op IPC to browser process if child
knows trial is activated. Still, this came up during design review and
it doesn't hurt to use these.

BUG=673042
==========

[Alexei Svitkine (slow), 2016-12-12 18:14:24]

Description was changed from

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data and it's ok for child processes to observe a stale value because
it's only used to avoid sending a no-op IPC to browser process if child
knows trial is activated. Still, this came up during design review and
it doesn't hurt to use these.

BUG=673042
==========

to

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data.

It does not need synchronization or ordering constraints because it's
ok for child processes to observe a stale value because it's only used
to avoid sending a no-op IPC to browser process if child knows trial is
activated. Still, this came up during design review and it doesn't hurt
to use these.

BUG=673042
==========

[Alexei Svitkine (slow), 2016-12-12 18:14:32]

asvitkine@chromium.org changed reviewers:
+ bcwhite@chromium.org

[bcwhite, 2016-12-13 13:38:50]

Comment that it's required to prevent the compiler from doing unexpected
optimizations because it thinks that only one thread is accessing that memory
address.

https://codereview.chromium.org/2570473003/diff/20001/base/metrics/field_tria...
File base/metrics/field_trial.cc (right):

https://codereview.chromium.org/2570473003/diff/20001/base/metrics/field_tria...
base/metrics/field_trial.cc:1089: new_entry->activated = prev_entry->activated;
These need to be accessed using atomic ops or you're likely to get a "linter" or
"sanitizer" complaining about non-atomic operations on an atomic variable,
either now or in the future.

https://codereview.chromium.org/2570473003/diff/20001/base/metrics/field_tria...
base/metrics/field_trial.cc:1285: entry->activated = trial_state.activated;
Still need to so "linters" and "sanitizers" won't complain.

[Alexei Svitkine (slow), 2016-12-13 15:26:05]

https://codereview.chromium.org/2570473003/diff/20001/base/metrics/field_tria...
File base/metrics/field_trial.cc (right):

https://codereview.chromium.org/2570473003/diff/20001/base/metrics/field_tria...
base/metrics/field_trial.cc:1089: new_entry->activated = prev_entry->activated;
On 2016/12/13 13:38:50, bcwhite wrote:
> These need to be accessed using atomic ops or you're likely to get a "linter"
or
> "sanitizer" complaining about non-atomic operations on an atomic variable,
> either now or in the future.

Done.

https://codereview.chromium.org/2570473003/diff/20001/base/metrics/field_tria...
base/metrics/field_trial.cc:1285: entry->activated = trial_state.activated;
On 2016/12/13 13:38:50, bcwhite wrote:
> Still need to so "linters" and "sanitizers" won't complain.

Done.

[bcwhite, 2016-12-13 15:50:00]

lgtm

[Alexei Svitkine (slow), 2016-12-13 17:15:14]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2016-12-13 17:15:41]

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-12-13 18:38:00]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481649314790360,
"parent_rev": "5a5e0b5cb259b812890b71a908d90bb975fcff39", "commit_rev":
"f272f67eec16326deeee11f7a7bb9190ae91ea4b"}

[commit-bot: I haz the power, 2016-12-13 18:38:30]

Description was changed from

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data.

It does not need synchronization or ordering constraints because it's
ok for child processes to observe a stale value because it's only used
to avoid sending a no-op IPC to browser process if child knows trial is
activated. Still, this came up during design review and it doesn't hurt
to use these.

BUG=673042
==========

to

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data.

It does not need synchronization or ordering constraints because it's
ok for child processes to observe a stale value because it's only used
to avoid sending a no-op IPC to browser process if child knows trial is
activated. Still, this came up during design review and it doesn't hurt
to use these.

BUG=673042

Review-Url: https://codereview.chromium.org/2570473003
==========

[commit-bot: I haz the power, 2016-12-13 18:38:31]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-12-13 18:42:28]

Description was changed from

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data.

It does not need synchronization or ordering constraints because it's
ok for child processes to observe a stale value because it's only used
to avoid sending a no-op IPC to browser process if child knows trial is
activated. Still, this came up during design review and it doesn't hurt
to use these.

BUG=673042

Review-Url: https://codereview.chromium.org/2570473003
==========

to

==========
NoBarrier_Load()/NoBarrier_Store() for manipulating activated field.

These ops guarantee atomicity of the write/read (i.e. not partial), but
do not put constraints around synchronization and ordering.

This is mostly academic since the field's value is either 0 or 1, so
currently is not susceptible to issues from partial writes of the
data.

It does not need synchronization or ordering constraints because it's
ok for child processes to observe a stale value because it's only used
to avoid sending a no-op IPC to browser process if child knows trial is
activated. Still, this came up during design review and it doesn't hurt
to use these.

BUG=673042

Committed: https://crrev.com/8095c757053bd2bc15b5b2570362587ba38c5779
Cr-Commit-Position: refs/heads/master@{#438235}
==========

[commit-bot: I haz the power, 2016-12-13 18:42:29]

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