allocator: add use_experimental_allocator_shim build flag

allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

Design doc: http://bit.ly/allocator-shim

BUG=550886
Committed: https://crrev.com/3217078aa86523fb341e0cf2b0c49d55434b534a
Cr-Commit-Position: refs/heads/master@{#374449}

[Primiano Tucci (use gerrit), 2016-02-08 15:25:13]

Description was changed from

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shimallocator: add use_experimental_allocator_shim
build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

BUG=550886
==========

to

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

BUG=550886
==========

[Primiano Tucci (use gerrit), 2016-02-08 17:37:05]

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

[Will Harris, 2016-02-08 19:23:55]

lgtm for base/allocator and third_party/tcmalloc

https://codereview.chromium.org/1678893002/diff/20001/third_party/tcmalloc/ch...
File third_party/tcmalloc/chromium/src/libc_override.h (right):

https://codereview.chromium.org/1678893002/diff/20001/third_party/tcmalloc/ch...
third_party/tcmalloc/chromium/src/libc_override.h:64: static void
ReplaceSystemAlloc() {}
add some comment here. it's a bit subtle what this is doing.

[Primiano Tucci (use gerrit), 2016-02-08 19:30:54]

Description was changed from

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

BUG=550886
==========

to

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

Design doc: http://bit.ly/allocator-shim

BUG=550886
==========

[Primiano Tucci (use gerrit), 2016-02-08 19:30:54]

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

[Primiano Tucci (use gerrit), 2016-02-08 19:32:35]

+brettw for build/config/allocator.gni

[Primiano Tucci (use gerrit), 2016-02-09 11:38:53]

https://codereview.chromium.org/1678893002/diff/20001/third_party/tcmalloc/ch...
File third_party/tcmalloc/chromium/src/libc_override.h (right):

https://codereview.chromium.org/1678893002/diff/20001/third_party/tcmalloc/ch...
third_party/tcmalloc/chromium/src/libc_override.h:64: static void
ReplaceSystemAlloc() {}
On 2016/02/08 19:23:55, Will Harris (ooo until 25 Feb) wrote:
> add some comment here. it's a bit subtle what this is doing.

Done.

[Primiano Tucci (use gerrit), 2016-02-09 13:47:47]

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

[Primiano Tucci (use gerrit), 2016-02-09 13:47:47]

+thakis for  build/config/allocator.gni

[scottmg, 2016-02-09 19:46:16]

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

[scottmg, 2016-02-09 19:46:17]

build/config/allocator.gni lgtm

[Primiano Tucci (use gerrit), 2016-02-09 19:48:35]

The CQ bit was checked by primiano@chromium.org

[Primiano Tucci (use gerrit), 2016-02-09 19:48:35]

The patchset sent to the CQ was uploaded after l-g-t-m from wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1678893002/#ps40001
(title: "add comment")

[commit-bot: I haz the power, 2016-02-09 19:51:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1678893002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1678893002/40001

[Nico, 2016-02-09 20:14:23]

https://codereview.chromium.org/1678893002/diff/40001/third_party/tcmalloc/ch...
File third_party/tcmalloc/chromium/src/libc_override.h (right):

https://codereview.chromium.org/1678893002/diff/40001/third_party/tcmalloc/ch...
third_party/tcmalloc/chromium/src/libc_override.h:70: static void
ReplaceSystemAlloc() {}
Which files include this? Defining static functions inline without marking them
inline generally isn't a good idea (yes, this file does this already) -- you'll
end up with one copy of the function in every TU including the header. This
should instead be marked inline, not static. (If the header needs to work in
both .c and .cc TUs, a little more work is needed.)

[Nico, 2016-02-09 20:14:41]

(doesn't have to block cq, just wanted to point that out)

[commit-bot: I haz the power, 2016-02-09 20:22:25]

Description was changed from

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

Design doc: http://bit.ly/allocator-shim

BUG=550886
==========

to

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

Design doc: http://bit.ly/allocator-shim

BUG=550886
==========

[commit-bot: I haz the power, 2016-02-09 20:22:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-09 20:23:19]

Description was changed from

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

Design doc: http://bit.ly/allocator-shim

BUG=550886
==========

to

==========
allocator: add use_experimental_allocator_shim build flag

This CL introduces a new GYP/GN build flag
use_experimental_allocator_shim (default: false).
After this CL, the only side-effect of that flag is that tcmalloc
does not override anymore the libc symbols (malloc, new etc.).
This is in preparation of a unified shim layer, which will come
separately in upcoming CLs, which will take care of overriding
those symbols in Chrome.

Design doc: http://bit.ly/allocator-shim

BUG=550886

Committed: https://crrev.com/3217078aa86523fb341e0cf2b0c49d55434b534a
Cr-Commit-Position: refs/heads/master@{#374449}
==========

[commit-bot: I haz the power, 2016-02-09 20:23:20]

Patchset 3 (id:??) landed as
https://crrev.com/3217078aa86523fb341e0cf2b0c49d55434b534a
Cr-Commit-Position: refs/heads/master@{#374449}

[Primiano Tucci (use gerrit), 2016-02-09 20:34:23]

https://codereview.chromium.org/1678893002/diff/40001/third_party/tcmalloc/ch...
File third_party/tcmalloc/chromium/src/libc_override.h (right):

https://codereview.chromium.org/1678893002/diff/40001/third_party/tcmalloc/ch...
third_party/tcmalloc/chromium/src/libc_override.h:70: static void
ReplaceSystemAlloc() {}
On 2016/02/09 20:14:23, Nico wrote:
> Which files include this? Defining static functions inline without marking
them
> inline generally isn't a good idea (yes, this file does this already) --
you'll
> end up with one copy of the function in every TU including the header. 

So I think that the deal is that this header is a special snowflake and is not
really a header. This is meant to be included only once by tcmalloc.cc.
I think the only reason why this is a .h is because if you name it .cc and add
to the build files, it will be treated as a TU and built independently, which
you do NOT want here either, because this file requires to see the *definitions*
of the tc_* symbols from tcmalloc.cc .
In turn, the reason why this file needs to see the definitions is because of the
attribute(alias) black magic used by the includes below (see
override_gcc_and_weak as an example). attribute(alias) can be used only when the
definition is seen in the same TU.

In other words, this is really a part of tcmalloc.cc,
think to this as a C# .partial.cs file
My guess is that the original authors did split out this header for code
readability convenience (for some definition of "convenience" given the black
magic all around here)