Cleanup allocator_shim.cc from ENABLE_DYNAMIC_ALLOCATOR_SWITCHING

Cleanup allocator_shim.cc from ENABLE_DYNAMIC_ALLOCATOR_SWITCHING

Whether to use this feature is decided in the gyp files.
The flag is defined unconditionally so it does not makes sense
to check it in #ifdef conditions.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233756

[kbalazs, 2013-10-31 19:01:26]

About try bot results: failing tests on windows are
PageCyclerUnitTest.testCacheHandled and
SmoothnessMetricUnitTest.testCalcResultsTraceEvents, seems pretty much unrelated
(anyway this patch only removes dead code so I only really wanted to try the
build).

[jar (doing other things), 2013-11-01 00:37:50]

lgtm

[commit-bot: I haz the power, 2013-11-01 19:11:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/1

[commit-bot: I haz the power, 2013-11-01 20:12:17]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[kbalazs, 2013-11-04 17:01:30]

Oops, I skipped one |#if ENABLE_DYNAMIC_ALLOCATOR_SWITCHING|.

[commit-bot: I haz the power, 2013-11-04 17:40:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/290001

[commit-bot: I haz the power, 2013-11-04 18:43:58]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-04 19:40:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/530001

[commit-bot: I haz the power, 2013-11-04 20:26:31]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-11-04 22:46:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/510002

[commit-bot: I haz the power, 2013-11-05 01:13:52]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-05 23:32:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/950001

[commit-bot: I haz the power, 2013-11-06 01:04:10]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-06 01:24:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/1200001

[commit-bot: I haz the power, 2013-11-06 02:49:18]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-06 22:46:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/1200001

[commit-bot: I haz the power, 2013-11-06 23:29:07]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Dai Mikurube (NOT FULLTIME), 2013-11-07 02:37:42]

http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/220901...

FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma\gomacc.exe
"c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\cl.exe" /nologo
/showIncludes /FC @obj\base\allocator\allocator.allocator_shim.obj.rsp /c
..\..\base\allocator\allocator_shim.cc
/Foobj\base\allocator\allocator.allocator_shim.obj
/Fdobj\base\allocator\allocator.pdb 
e:\b\build\slave\win\build\src\base\allocator\allocator_shim.cc(221) :error
C3861: 'NOTREACHED': identifier not found

It looks like that the compile error is from your change. You need to update
your patch.

[kbalazs, 2013-11-07 16:56:34]

On 2013/11/07 02:37:42, Dai Mikurube wrote:
>
http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/220901...
> 
> FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma\gomacc.exe
> "c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\cl.exe" /nologo
> /showIncludes /FC @obj\base\allocator\allocator.allocator_shim.obj.rsp /c
> ..\..\base\allocator\allocator_shim.cc
> /Foobj\base\allocator\allocator.allocator_shim.obj
> /Fdobj\base\allocator\allocator.pdb 
> e:\b\build\slave\win\build\src\base\allocator\allocator_shim.cc(221) :error
> C3861: 'NOTREACHED': identifier not found
> 
> It looks like that the compile error is from your change. You need to update
> your patch.

Yes I know, sorry for the noise, I just don't get it. My idea was that
base/logging.h is somehow not included through the other includes (although it
is included on linux) so I added the include. base/logging.h defines NOTREACHED
here:
https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&q=N...
This is an unconditional block (not ifdefed). If you have a quick idea what am I
not seeing, please help me out.

[jar (doing other things), 2013-11-07 19:22:36]

https://codereview.chromium.org/47873005/diff/1200001/base/allocator/allocato...
File base/allocator/allocator_shim.cc (right):

https://codereview.chromium.org/47873005/diff/1200001/base/allocator/allocato...
base/allocator/allocator_shim.cc:221: NOTREACHED();
Although this is a reasonable debug check.... it is a change in semantics.

Worse yet, in general, the allocator code generally avoids using an CHECK()
style code (which can induce allocations... <oops>), and instead uses extremely
simplistics ASSERT() implementations, which crash with no potential allocation
calls.  

If you are finding this inclusion problematic, I'd suggest either:
a) Don't include logging.h.... don't use NOTREACHED

b) Leave the code as it was, using the default to TCMALLOC, and don't worry
about the error where the case is not specified as you expect.

I'd probably argue for the latter, as it would have no chance to impact on
performace.

[kbalazs, 2013-11-07 21:33:33]

Updated to keep the original structure in _msize. Good point about not using
DCHECK in allocator. (I still not get why was the build error though...)

[commit-bot: I haz the power, 2013-11-07 22:56:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/47873005/1470001

[commit-bot: I haz the power, 2013-11-08 01:44:11]

Change committed as 233756