Use use_allocator instead of android_use_tcmalloc to switch the allocator.

Use use_allocator instead of android_use_tcmalloc to switch the allocator.

See https://crrev.com/177353002 and the bug for the details.

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

[Dai Mikurube (NOT FULLTIME), 2014-03-20 15:56:32]

Hi Primiano and Mikhail,

Could you take a look, and try this patch for Android when you have time?

It's the new allocator gyp switch discussed at http://crbug.com/345554. Linux
has already been switched. It switches Android.

I just tried to write a patchset, but I don't have an Android development
environment here...

[Dai Mikurube (NOT FULLTIME), 2014-03-20 15:57:56]

Please note that android_use_tcmalloc and these complex expressions are finally
removed. It's just a transition period. (Discussed at
https://crrev.com/177353002.)

[mnaganov (inactive), 2014-03-20 16:45:28]

On 2014/03/20 15:56:32, Dai Mikurube wrote:
> Hi Primiano and Mikhail,
> 
> Could you take a look, and try this patch for Android when you have time?
> 
> It's the new allocator gyp switch discussed at http://crbug.com/345554. Linux
> has already been switched. It switches Android.
> 
> I just tried to write a patchset, but I don't have an Android development
> environment here...

I'm sorry, I'm not very much familiar with this subject, so I can't say anything
about this change. Let's wait for Primiano's review.

[Dai Mikurube (NOT FULLTIME), 2014-03-20 16:53:06]

On 2014/03/20 16:45:28, Mikhail Naganov (Cr) wrote:
> On 2014/03/20 15:56:32, Dai Mikurube wrote:
> > Hi Primiano and Mikhail,
> > 
> > Could you take a look, and try this patch for Android when you have time?
> > 
> > It's the new allocator gyp switch discussed at http://crbug.com/345554.
Linux
> > has already been switched. It switches Android.
> > 
> > I just tried to write a patchset, but I don't have an Android development
> > environment here...
> 
> I'm sorry, I'm not very much familiar with this subject, so I can't say
anything
> about this change. Let's wait for Primiano's review.

Sure, no problem. I wanted to let you know about this change since you added
android_use_tcmalloc in android_webview.gyp.

[mnaganov (inactive), 2014-03-20 18:29:26]

On 2014/03/20 16:53:06, Dai Mikurube wrote:
> On 2014/03/20 16:45:28, Mikhail Naganov (Cr) wrote:
> > On 2014/03/20 15:56:32, Dai Mikurube wrote:
> > > Hi Primiano and Mikhail,
> > > 
> > > Could you take a look, and try this patch for Android when you have time?
> > > 
> > > It's the new allocator gyp switch discussed at http://crbug.com/345554.
> Linux
> > > has already been switched. It switches Android.
> > > 
> > > I just tried to write a patchset, but I don't have an Android development
> > > environment here...
> > 
> > I'm sorry, I'm not very much familiar with this subject, so I can't say
> anything
> > about this change. Let's wait for Primiano's review.
> 
> Sure, no problem. I wanted to let you know about this change since you added
> android_use_tcmalloc in android_webview.gyp.

Right, I already forgot about that :) What I mean, is that I did not follow the
recent allocator-related discussions, so I'm not entirely understanding the
change.

[Primiano Tucci (use gerrit), 2014-03-24 09:40:37]

Sorry for the delay guys, I was OOO in these days.
If Mikhail agrees, I think we could remove entirely the allocator-related gyp
sections for WebView.
My rationale is: conversely to what happens to Clank, recompiling the
libwebviewchromium.so with a custom allocator provides incomplete results: we
(non-statically) link against some system libraries (e.g. skia), that means that
any allocations made by (system) skia wouldn't be traced. Also it will not help
to detect other memory leaks/bloats which are caused by the embedder application
itself (we can recompile our webview .so with tcmalloc, but not the original
application).
As I was anticipating, I am working on the memory_inspector tool to achieve this
on Android using the instrumented system libc malloc (which doesn't require
recompilation) and my primary focus is obviously WebView :)

If there isn't anything else I'm missing, I think it would be just better if we
reduce the complexity of our GYP file by just removing these references to the
allocator entirely, which would also mean on problem less for you to think about
:)

[mnaganov (inactive), 2014-03-24 09:56:09]

On 2014/03/24 09:40:37, Primiano Tucci wrote:
> Sorry for the delay guys, I was OOO in these days.
> If Mikhail agrees, I think we could remove entirely the allocator-related gyp
> sections for WebView.
> My rationale is: conversely to what happens to Clank, recompiling the
> libwebviewchromium.so with a custom allocator provides incomplete results: we
> (non-statically) link against some system libraries (e.g. skia), that means
that
> any allocations made by (system) skia wouldn't be traced. Also it will not
help
> to detect other memory leaks/bloats which are caused by the embedder
application
> itself (we can recompile our webview .so with tcmalloc, but not the original
> application).
> As I was anticipating, I am working on the memory_inspector tool to achieve
this
> on Android using the instrumented system libc malloc (which doesn't require
> recompilation) and my primary focus is obviously WebView :)
> 
> If there isn't anything else I'm missing, I think it would be just better if
we
> reduce the complexity of our GYP file by just removing these references to the
> allocator entirely, which would also mean on problem less for you to think
about
> :)

I have no objections to remove allocator-related stuff from WebView gyp.
The introduction of the allocator selection was an exploratory movement,
and I was following the Chromium-wide approach for enabling DMProf for WebView.
As Primiano has demonstrated us a better approach, I think we can retire
my experimental stuff.

[Dai Mikurube (NOT FULLTIME), 2014-03-24 10:17:05]

On 2014/03/24 09:56:09, Mikhail Naganov (Cr) wrote:
> On 2014/03/24 09:40:37, Primiano Tucci wrote:
> > Sorry for the delay guys, I was OOO in these days.
> > If Mikhail agrees, I think we could remove entirely the allocator-related
gyp
> > sections for WebView.
> > My rationale is: conversely to what happens to Clank, recompiling the
> > libwebviewchromium.so with a custom allocator provides incomplete results:
we
> > (non-statically) link against some system libraries (e.g. skia), that means
> that
> > any allocations made by (system) skia wouldn't be traced. Also it will not
> help
> > to detect other memory leaks/bloats which are caused by the embedder
> application
> > itself (we can recompile our webview .so with tcmalloc, but not the original
> > application).
> > As I was anticipating, I am working on the memory_inspector tool to achieve
> this
> > on Android using the instrumented system libc malloc (which doesn't require
> > recompilation) and my primary focus is obviously WebView :)
> > 
> > If there isn't anything else I'm missing, I think it would be just better if
> we
> > reduce the complexity of our GYP file by just removing these references to
the
> > allocator entirely, which would also mean on problem less for you to think
> about
> > :)
> 
> I have no objections to remove allocator-related stuff from WebView gyp.
> The introduction of the allocator selection was an exploratory movement,
> and I was following the Chromium-wide approach for enabling DMProf for
WebView.
> As Primiano has demonstrated us a better approach, I think we can retire
> my experimental stuff.

Okay. Primiano, do you mean removing the entire section of
'android_webview_build==1 and android_use_tcmalloc==1'
immediately in this patchset? If so, I'd update the patchset soon.

[Dai Mikurube (NOT FULLTIME), 2014-03-27 05:27:07]

Primiano,

Could you take a look_ I removed my change in android_webview.gypi to catch up
to your revert in r259209.

[Primiano Tucci (use gerrit), 2014-03-27 10:05:38]

It looks consistent to me to the other changes you made on allocator. LGTM (I'm
not an owner of those files, though)

[Dai Mikurube (NOT FULLTIME), 2014-03-27 10:25:28]

The CQ bit was checked by dmikurube@chromium.org

[commit-bot: I haz the power, 2014-03-27 10:25:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmikurube@chromium.org/206413006/80001

[Dai Mikurube (NOT FULLTIME), 2014-03-27 10:26:13]

* is actually na OWNER of these gypi-s. :) Committing...

[commit-bot: I haz the power, 2014-03-27 16:05:47]

Change committed as 259894