Remove some LazyInstance use in base/

Remove some LazyInstance use in base/

R=mark@chromium.org
TBR=jam@chromium.org
BUG=686866, 587210, 686730
Review-Url: https://codereview.chromium.org/2667513003
Cr-Commit-Position: refs/heads/master@{#447558}
Committed: https://chromium.googlesource.com/chromium/src/+/6ece5aec616aa4d8d06359e426ac154904aabe5b

[scottmg, 2017-01-30 22:08:11]

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

[commit-bot: I haz the power, 2017-01-30 22:09:26]

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

[scottmg, 2017-01-30 22:09:37]

Description was changed from

==========
Remove use of LazyInstance in base/

BUG=686866, 587210, 686730
==========

to

==========
Remove some LazyInstance use in base/

BUG=686866, 587210, 686730
==========

[scottmg, 2017-01-30 22:15:29]

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

[commit-bot: I haz the power, 2017-01-30 22:15:39]

Dry run: 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, 2017-01-30 22:24:29]

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

[commit-bot: I haz the power, 2017-01-30 22:24:29]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[scottmg, 2017-01-30 22:31:39]

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

[commit-bot: I haz the power, 2017-01-30 22:32:39]

Dry run: 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, 2017-01-30 22:42:28]

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

[commit-bot: I haz the power, 2017-01-30 22:42:29]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[scottmg, 2017-01-30 23:38:24]

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

[commit-bot: I haz the power, 2017-01-30 23:39:04]

Dry run: 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, 2017-01-30 23:47:32]

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

[commit-bot: I haz the power, 2017-01-30 23:47:33]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[scottmg, 2017-01-30 23:51:14]

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

[commit-bot: I haz the power, 2017-01-30 23:51:49]

Dry run: 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, 2017-01-31 00:06:38]

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

[commit-bot: I haz the power, 2017-01-31 00:06:39]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[scottmg, 2017-01-31 00:07:47]

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

[commit-bot: I haz the power, 2017-01-31 00:08:18]

Dry run: 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, 2017-01-31 00:39:53]

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

[commit-bot: I haz the power, 2017-01-31 00:39:54]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[scottmg, 2017-01-31 00:42:09]

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

[commit-bot: I haz the power, 2017-01-31 00:42:29]

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

[Mark Mentovai, 2017-01-31 00:52:09]

I know you didn’t request review, I’m just commenting because you linked to this
from https://codereview.chromium.org/2667723002/.

https://codereview.chromium.org/2667513003/diff/120001/base/cpu.cc
File base/cpu.cc (right):

https://codereview.chromium.org/2667513003/diff/120001/base/cpu.cc#newcode220
base/cpu.cc:220: static auto lazy_cpuinfo = new LazyCpuInfoValue();
I don’t know how much digging you want to do at each of these use sites, but it
occurs to me that in cases like this, we can simplify a little more:

// in place of class LazyCpuInfoValue
std::string CpuInfoBrandInternal();
std::string* CpuInfoBrand() {
  static std::string* value = new std::string(CpuInfoBrandInternal();
  return value;
}

  // right here
  cpu_brand_.assign(*CpuInfoBrand());

Or another thing I’ve done (I think we’ve done it together actually) has been to
get rid of the “internal” function and use a lambda.

std::string* CpuInfoBrand() {
  static std::string* value = [](){
    return new std::string(/* … */);
  }();
}

Or, in this case, since it’s only used in one place, get rid of both functions
and just put the static std::string* in one place, right here.

Different levels of improvement, and if it’s like 1,000 use sites, I don’t know
how intimate you’ll want to get with each one, but it’d be nice to not just get
rid of the legacy of LazyInstance but to also overcome design decisions that it
imposed on the code too.

[scottmg, 2017-01-31 01:03:05]

On 2017/01/31 00:52:09, Mark Mentovai wrote:
> I know you didn’t request review, I’m just commenting because you linked to
this
> from https://codereview.chromium.org/2667723002/.
> 
> https://codereview.chromium.org/2667513003/diff/120001/base/cpu.cc
> File base/cpu.cc (right):
> 
> https://codereview.chromium.org/2667513003/diff/120001/base/cpu.cc#newcode220
> base/cpu.cc:220: static auto lazy_cpuinfo = new LazyCpuInfoValue();
> I don’t know how much digging you want to do at each of these use sites, but
it
> occurs to me that in cases like this, we can simplify a little more:
> 
> // in place of class LazyCpuInfoValue
> std::string CpuInfoBrandInternal();
> std::string* CpuInfoBrand() {
>   static std::string* value = new std::string(CpuInfoBrandInternal();
>   return value;
> }
> 
>   // right here
>   cpu_brand_.assign(*CpuInfoBrand());
> 
> Or another thing I’ve done (I think we’ve done it together actually) has been
to
> get rid of the “internal” function and use a lambda.
> 
> std::string* CpuInfoBrand() {
>   static std::string* value = [](){
>     return new std::string(/* … */);
>   }();
> }
> 
> Or, in this case, since it’s only used in one place, get rid of both functions
> and just put the static std::string* in one place, right here.
> 
> Different levels of improvement, and if it’s like 1,000 use sites, I don’t
know
> how intimate you’ll want to get with each one, but it’d be nice to not just
get
> rid of the legacy of LazyInstance but to also overcome design decisions that
it
> imposed on the code too.

Yeah, good point. There are ~1000, so maybe the best thing to do is a first pass
only doing the more obvious/simple locations, and then investigate the uses that
would benefit from more rework like this one.

[commit-bot: I haz the power, 2017-01-31 01:15:59]

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

[commit-bot: I haz the power, 2017-01-31 01:16:00]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[scottmg, 2017-01-31 01:21:16]

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

[commit-bot: I haz the power, 2017-01-31 01:21:36]

Dry run: 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, 2017-01-31 01:23:49]

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

[commit-bot: I haz the power, 2017-01-31 01:23:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[scottmg, 2017-01-31 01:59:04]

Description was changed from

==========
Remove some LazyInstance use in base/

BUG=686866, 587210, 686730
==========

to

==========
Remove some LazyInstance use in base/

BUG=686866, 587210, 686730
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2017-01-31 01:59:15]

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

[commit-bot: I haz the power, 2017-01-31 01:59:53]

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

[scottmg, 2017-01-31 02:16:07]

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

[commit-bot: I haz the power, 2017-01-31 02:16:36]

Dry run: 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, 2017-01-31 02:33:49]

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

[commit-bot: I haz the power, 2017-01-31 02:33:50]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[scottmg, 2017-01-31 02:36:54]

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

[commit-bot: I haz the power, 2017-01-31 02:37:17]

Dry run: 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, 2017-01-31 03:02:35]

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

[commit-bot: I haz the power, 2017-01-31 03:02:36]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[scottmg, 2017-01-31 03:31:54]

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

[commit-bot: I haz the power, 2017-01-31 03:32:41]

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

[scottmg, 2017-01-31 05:00:58]

scottmg@chromium.org changed reviewers:
+ mark@chromium.org

[scottmg, 2017-01-31 05:00:59]

Updated cpu.cc to match one of your suggestions (inline in the one use seemed
too bulky) and trimmed the CL down to only relatively simple switches from
LazyInstance<>. Please take a look when you have a chance.

[commit-bot: I haz the power, 2017-01-31 05:33:08]

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

[commit-bot: I haz the power, 2017-01-31 05:33:09]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Mark Mentovai, 2017-01-31 14:37:01]

LGTM

https://codereview.chromium.org/2667513003/diff/260001/base/debug/close_handl...
File base/debug/close_handle_hook_win.cc (left):

https://codereview.chromium.org/2667513003/diff/260001/base/debug/close_handl...
base/debug/close_handle_hook_win.cc:206: base::LazyInstance<HandleHooks> g_hooks
= LAZY_INSTANCE_INITIALIZER;
This one wasn’t “leaky”, but I think that was an omission.

https://codereview.chromium.org/2667513003/diff/260001/base/win/message_windo...
File base/win/message_window.cc (left):

https://codereview.chromium.org/2667513003/diff/260001/base/win/message_windo...
base/win/message_window.cc:35: static LazyInstance<MessageWindow::WindowClass>
g_window_class =
This wasn’t leaky, and WindowClass has a destructor that does more than nothing.
How do you want to handle it?

https://codereview.chromium.org/2667513003/diff/260001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2667513003/diff/260001/base/win/win_util.cc#n...
base/win/win_util.cc:542: static auto lazy_is_user32_and_gdi32_available =
This is an excellent candidate for the same kind of update that you did in
cpu.cc. In this case, I think it’s worthwhile to move everything into this
function.

[scottmg, 2017-01-31 18:18:06]

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

[commit-bot: I haz the power, 2017-01-31 18:18:41]

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

[scottmg, 2017-01-31 18:19:16]

Thanks.

https://codereview.chromium.org/2667513003/diff/260001/base/debug/close_handl...
File base/debug/close_handle_hook_win.cc (left):

https://codereview.chromium.org/2667513003/diff/260001/base/debug/close_handl...
base/debug/close_handle_hook_win.cc:206: base::LazyInstance<HandleHooks> g_hooks
= LAZY_INSTANCE_INITIALIZER;
On 2017/01/31 14:37:01, Mark Mentovai wrote:
> This one wasn’t “leaky”, but I think that was an omission.

Yes, I thought it was accidental too. I just now removed RemoveHandleHooks()
which was empty and used only in one location in any case, as that would
conceivably have been a location where HandleHooks might have been cleaned up.

https://codereview.chromium.org/2667513003/diff/260001/base/win/message_windo...
File base/win/message_window.cc (left):

https://codereview.chromium.org/2667513003/diff/260001/base/win/message_windo...
base/win/message_window.cc:35: static LazyInstance<MessageWindow::WindowClass>
g_window_class =
On 2017/01/31 14:37:01, Mark Mentovai wrote:
> This wasn’t leaky, and WindowClass has a destructor that does more than
nothing.
> How do you want to handle it?

For general process shutdown, there's little point to UnregisterClass()ing, but
I guess the DCHECK potentially has some minor value in detecting leaks.

Hmm... I guess we might want a general helper function that either does both the
new'ing and OnExit() registration. But here, I think I'll just call
AtExitManager::RegisterCallback() for now, until we see how many non-leaky
things we truly want.

https://codereview.chromium.org/2667513003/diff/260001/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2667513003/diff/260001/base/win/win_util.cc#n...
base/win/win_util.cc:542: static auto lazy_is_user32_and_gdi32_available =
On 2017/01/31 14:37:01, Mark Mentovai wrote:
> This is an excellent candidate for the same kind of update that you did in
> cpu.cc. In this case, I think it’s worthwhile to move everything into this
> function.

Done.

[scottmg, 2017-01-31 18:24:12]

Description was changed from

==========
Remove some LazyInstance use in base/

BUG=686866, 587210, 686730
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Remove some LazyInstance use in base/

R=mark@chromium.org
TBR=jam@chromium.org
BUG=686866, 587210, 686730
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2017-01-31 18:24:12]

scottmg@chromium.org changed reviewers:
+ jam@chromium.org

[Mark Mentovai, 2017-01-31 18:25:26]

LGTM

https://codereview.chromium.org/2667513003/diff/280001/base/debug/close_handl...
File base/debug/close_handle_hook_win.cc (right):

https://codereview.chromium.org/2667513003/diff/280001/base/debug/close_handl...
base/debug/close_handle_hook_win.cc:233: void HandleHooks::Unpatch() {
Should we lose this dead code now too then?

[scottmg, 2017-01-31 18:40:51]

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

[commit-bot: I haz the power, 2017-01-31 18:41:35]

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

[scottmg, 2017-01-31 18:42:02]

(Also fixed ~WindowClass() call).

TBR jam for #include fixes outside of base/.

https://codereview.chromium.org/2667513003/diff/280001/base/debug/close_handl...
File base/debug/close_handle_hook_win.cc (right):

https://codereview.chromium.org/2667513003/diff/280001/base/debug/close_handl...
base/debug/close_handle_hook_win.cc:233: void HandleHooks::Unpatch() {
On 2017/01/31 18:25:26, Mark Mentovai wrote:
> Should we lose this dead code now too then?

Done.

[scottmg, 2017-01-31 18:42:23]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2017-01-31 18:42:45]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2017-01-31 18:42:46]

The patchset sent to the CQ was uploaded after l-g-t-m from mark@chromium.org
Link to the patchset: https://codereview.chromium.org/2667513003/#ps300001
(title: ".")

[commit-bot: I haz the power, 2017-01-31 18:43:56]

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, 2017-01-31 19:58:36]

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

[commit-bot: I haz the power, 2017-01-31 19:58:37]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[scottmg, 2017-01-31 21:27:43]

https://codereview.chromium.org/2667513003/diff/300001/base/win/message_windo...
File base/win/message_window.cc (right):

https://codereview.chromium.org/2667513003/diff/300001/base/win/message_windo...
base/win/message_window.cc:41: AtExitManager::RegisterCallback(deleter, ret);
This "quick fix" is incorrect because in tests there's an pseudo AtExitManager
that runs and even though the process isn't shutting down we run the AtExits and
reset lazy instances.

I'm going to revert message_window.* out of this CL for now. I think likely the
correct thing to do here is just make it truly leaky.

[scottmg, 2017-01-31 21:29:02]

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

[commit-bot: I haz the power, 2017-01-31 21:30:02]

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

[scottmg, 2017-01-31 21:56:13]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2017-01-31 21:56:19]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2017-01-31 21:56:20]

The patchset sent to the CQ was uploaded after l-g-t-m from mark@chromium.org
Link to the patchset: https://codereview.chromium.org/2667513003/#ps320001
(title: "no message_window")

[commit-bot: I haz the power, 2017-01-31 21:57:08]

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, 2017-02-01 00:00:18]

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

[commit-bot: I haz the power, 2017-02-01 00:00:19]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[scottmg, 2017-02-01 00:07:52]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2017-02-01 00:08:29]

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

[jam, 2017-02-01 01:13:43]

lgtm

[commit-bot: I haz the power, 2017-02-01 02:41:53]

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

[commit-bot: I haz the power, 2017-02-01 02:41:55]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[scottmg, 2017-02-01 18:16:18]

Description was changed from

==========
Remove some LazyInstance use in base/

R=mark@chromium.org
TBR=jam@chromium.org
BUG=686866, 587210, 686730
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Remove some LazyInstance use in base/

R=mark@chromium.org
TBR=jam@chromium.org
BUG=686866, 587210, 686730
==========

[scottmg, 2017-02-01 18:16:53]

On 2017/02/01 02:41:55, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

Removing linux_site_isolation from CQ bots because the bot seems broken.
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_iso...

Pinging site-isolation-dev@ to take a look.

[scottmg, 2017-02-01 18:17:36]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2017-02-01 18:18:18]

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, 2017-02-01 18:25:24]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1485973056237190,
"parent_rev": "7b1efd5dae551972821f2f6322a32b7a708de9bd", "commit_rev":
"6ece5aec616aa4d8d06359e426ac154904aabe5b"}

[commit-bot: I haz the power, 2017-02-01 18:26:04]

Description was changed from

==========
Remove some LazyInstance use in base/

R=mark@chromium.org
TBR=jam@chromium.org
BUG=686866, 587210, 686730
==========

to

==========
Remove some LazyInstance use in base/

R=mark@chromium.org
TBR=jam@chromium.org
BUG=686866, 587210, 686730

Review-Url: https://codereview.chromium.org/2667513003
Cr-Commit-Position: refs/heads/master@{#447558}
Committed:
https://chromium.googlesource.com/chromium/src/+/6ece5aec616aa4d8d06359e426ac...
==========

[commit-bot: I haz the power, 2017-02-01 18:26:05]

Committed patchset #17 (id:320001) as
https://chromium.googlesource.com/chromium/src/+/6ece5aec616aa4d8d06359e426ac...

[Primiano Tucci (use gerrit), 2017-02-07 16:20:35]

primiano@chromium.org changed reviewers:
+ primiano@chromium.org

[Primiano Tucci (use gerrit), 2017-02-07 16:20:36]

https://codereview.chromium.org/2667513003/diff/320001/base/trace_event/trace...
File base/trace_event/trace_log.cc (right):

https://codereview.chromium.org/2667513003/diff/320001/base/trace_event/trace...
base/trace_event/trace_log.cc:92: static auto filters = new
std::vector<std::unique_ptr<TraceEventFilter>>();
Nice, this one seems to have caused a 5% improvement of tracing macros in linux
(which is good)
https://chromeperf.appspot.com/report?sid=a9893b9f1441204d5064113eb8e6a806cf3...

I guess this mostly because of getting read of the acquire_load used by
lazy_instance and instead is performing an efficient thread-safe initialization
using some compiler builtin.
(Note: acquire-load on x86 should be "just a load" [1]. However, as I found out
in crbug.com/593344 today two full mfences are emitted because of
crbug.com/592903). Which also would explain why this improvement showed up only
on linux.

[1] https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html

[scottmg, 2017-02-07 18:11:35]

On 2017/02/07 16:20:36, Primiano Tucci wrote:
>
https://codereview.chromium.org/2667513003/diff/320001/base/trace_event/trace...
> File base/trace_event/trace_log.cc (right):
> 
>
https://codereview.chromium.org/2667513003/diff/320001/base/trace_event/trace...
> base/trace_event/trace_log.cc:92: static auto filters = new
> std::vector<std::unique_ptr<TraceEventFilter>>();
> Nice, this one seems to have caused a 5% improvement of tracing macros in
linux
> (which is good)
>
https://chromeperf.appspot.com/report?sid=a9893b9f1441204d5064113eb8e6a806cf3...
> 
> I guess this mostly because of getting read of the acquire_load used by
> lazy_instance and instead is performing an efficient thread-safe
initialization
> using some compiler builtin.
> (Note: acquire-load on x86 should be "just a load" [1]. However, as I found
out
> in crbug.com/593344 today two full mfences are emitted because of
> crbug.com/592903). Which also would explain why this improvement showed up
only
> on linux.
> 
> [1] https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html

Interesting, thanks for investigating.