Provide option to disable full DEBUG build on Android.

Provide option to disable full DEBUG build on Android.
    
V8 full DEBUG is too slow on Android. Disable it when android_full_debug==0.

BUG=none

[ulan, 2012-08-20 07:27:32]

There is no issue number for this CL.

Could you please give us more context? Are there tests timing out in debug mode?

Is this change only for android browser or do you want to upstream it
eventually?

In the latter case I wonder how it would affect android chrome.

[Xianzhu, 2012-08-20 17:34:34]

Thanks for the questions :)

The related changes are:
https://chromiumcodereview.appspot.com/10827273/
https://chromiumcodereview.appspot.com/10836323/
https://chromiumcodereview.appspot.com/10831381/

These CLs are 3 steps to change Android build configurations. Actually we never
used Debug configuration to build Android before. Instead we use Release
configuration with DCHECK enabled. With the changes, we "renamed" our original
"Release" to "Debug", while created a variable android_full_debug to build the
full debug version in case that someone needs it. The problem of full debug
version is that the binary runs too slow. For example, the WebKit layout tests
would take 4 hours to finish on a single device (vs 1.5 hours without v8 full
DEBUG). As we still want to run layout tests with DCHECKs and ASSERTs but don't
want it be so slow, we'd like to build Android Debug version without v8 full
DEBUG, while still allow the developer to enable full DEBUG with
andorid_full_debug variable.

We want to upstream this CL. Please let me know if anything should be corrected
to follow the process to make changes in v8.

[ulan, 2012-08-21 13:39:20]

Thank you for clarification, Xianzhu!

Not passing DEBUG by default in the debug build doesn't feel right to me.
Shouldn't the reduced debug build be opt-in instead of opt-out?

Another question: would it possible to keep the DEBUG flag intact and introduce
another preprocessor flag that disables some V8 checks in debug mode? I wonder
if conditionally disabling debug asserts and ZapRange, ZapBlock, Zap* functions
(and maybe some other debug function) would make the tests fast enough in debug
mode.

[Xianzhu, 2012-08-21 23:01:56]

On 2012/08/21 13:39:20, ulan wrote:
> Thank you for clarification, Xianzhu!
> 
> Not passing DEBUG by default in the debug build doesn't feel right to me.
> Shouldn't the reduced debug build be opt-in instead of opt-out?
> 

I just found I must define a default value for android_full_debug to support
standalone v8 build. The default value is 1 so from the v8 side, not passing
DEBUG is opt-in.

> Another question: would it possible to keep the DEBUG flag intact and
introduce
> another preprocessor flag that disables some V8 checks in debug mode? I wonder
> if conditionally disabling debug asserts and ZapRange, ZapBlock, Zap*
functions
> (and maybe some other debug function) would make the tests fast enough in
debug
> mode.

I tried but the result was weird: with the Zap functions disabled, our tests ran
even slower. And there seem so many places that I don't fully understand so I
abandoned the try :)

[Xianzhu, 2012-08-22 23:47:02]

On 2012/08/21 23:01:56, Xianzhu wrote:
> I tried but the result was weird: with the Zap functions disabled, our tests
ran
> even slower. And there seem so many places that I don't fully understand so I
> abandoned the try :)

I tried disabling ASSERT, but the result was still the same as with Zap
functions disabled. I also disabled some big Verify() functions and the result
was still the same. I'm profiling and will let you know the result.

[Xianzhu, 2012-08-23 06:34:33]

On 2012/08/22 23:47:02, Xianzhu wrote:
> I tried disabling ASSERT, but the result was still the same as with Zap
> functions disabled. I also disabled some big Verify() functions and the result
> was still the same. I'm profiling and will let you know the result.

Sorry the above statement was incorrect. Repeated test shows that disabling
ASSERT (as well as other checking macros in checks.h) got similar result as
disabling DEBUG.

[Xianzhu, 2012-08-23 07:02:52]

Is the opt-in method in Patch Set 3 acceptable? It seems that I won't have
enough affordable cycles to separate the slow asserts :(

[ulan, 2012-08-23 07:37:31]

On 2012/08/23 07:02:52, Xianzhu wrote:
> Is the opt-in method in Patch Set 3 acceptable? It seems that I won't have
> enough affordable cycles to separate the slow asserts :(

If there is a way to avoid hiding DEBUG in debug mode, I would prefer that way.

Disabling asserts like this doesn't work?

#if defined(DEBUG) && !defined(DISABLE_DEBUG_ASSERTS)
#define ASSERT_RESULT(expr)    CHECK(expr)
#define ASSERT(condition)      CHECK(condition)
#define ASSERT_EQ(v1, v2)      CHECK_EQ(v1, v2)
#define ASSERT_NE(v1, v2)      CHECK_NE(v1, v2)
#define ASSERT_GE(v1, v2)      CHECK_GE(v1, v2)
#define ASSERT_LT(v1, v2)      CHECK_LT(v1, v2)
#define ASSERT_LE(v1, v2)      CHECK_LE(v1, v2)
#define SLOW_ASSERT(condition) CHECK(!FLAG_enable_slow_asserts || (condition))
#else
#define ASSERT_RESULT(expr)    (expr)
#define ASSERT(condition)      ((void) 0)
#define ASSERT_EQ(v1, v2)      ((void) 0)
#define ASSERT_NE(v1, v2)      ((void) 0)
#define ASSERT_GE(v1, v2)      ((void) 0)
#define ASSERT_LT(v1, v2)      ((void) 0)
#define ASSERT_LE(v1, v2)      ((void) 0)
#define SLOW_ASSERT(condition) ((void) 0)
#endif

Note that we shouldn't disable the CHECK* macros since they are enabled even in
release mode.

[Xianzhu, 2012-08-23 16:27:34]

Yes, this is just the way I tested in #7, but I thought it was almost the same
as disabling DEBUG and thought you wouldn't accept it if you don't accept
disabling DEBUG.

Will make the change. Thanks.

[ulan, 2012-08-23 16:51:25]

Sorry for the confusion. Introducing DISABLE_DEBUG_ASSERTS is much better than
hiding DEBUG because we don't break implicit assumption about DEBUG being passed
in debug mode.

When you CL is ready, it is probably better to submit it upstream
(http://code.google.com/p/v8/wiki/Contributing) and then merge to chromium.

[Xianzhu, 2012-08-23 17:38:36]

On 2012/08/23 16:51:25, ulan wrote:
> When you CL is ready, it is probably better to submit it upstream
> (http://code.google.com/p/v8/wiki/Contributing) and then merge to chromium.

It says: "you need to get the depot_tools and follow these instructions on
requesting a review (using your V8 workspace instead of a chromium workspace". I
wonder how to create a V8 workspace with depot_tool. I added
    "custom_deps" : {
      "v8" : "http://v8.googlecode.com/svn/branches/bleeding_edge/",
    },
in .gclient of my chromium workspace. Is this OK?

[ulan, 2012-08-24 09:50:57]

Patch set 4 doesn't compile for me :( It produces a lot of "unused variable"
compiler errors. We need to go through all these places and guard them with the
!defined(DISABLE_DEBUG_ASSERTS). I opened V8 issue to track this
(http://code.google.com/p/v8/issues/detail?id=2304).

Since it will take time to fix this issue, let's for now land Patch set 3 in
chromium, so that you can run the layout tests.

Once the issue 2304 is fixed upstream, we can merge the proper solution into
chromium.

> I wonder how to create a V8 workspace with depot_tool.
First checkout V8:
svn checkout http://v8.googlecode.com/svn/branches/bleeding_edge/ v8
Then use gcl to upload.

If you use git and git cl, see http://code.google.com/p/v8/wiki/UsingGit

http://codereview.chromium.org/10823400/diff/6001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10823400/diff/6001/build/common.gypi#newcode350
build/common.gypi:350: # Disable full debug if we want a faster v8 in a debug
build.
Please add a comment:
# TODO(2304): pass DISABLE_DEBUG_ASSERT instead of hiding DEBUG.

[Xianzhu, 2012-08-24 16:10:47]

Thanks Ulan for verification and detailed instructions!

http://codereview.chromium.org/10823400/diff/6001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10823400/diff/6001/build/common.gypi#newcode350
build/common.gypi:350: # Disable full debug if we want a faster v8 in a debug
build.
On 2012/08/24 09:50:57, ulan wrote:
> Please add a comment:
> # TODO(2304): pass DISABLE_DEBUG_ASSERT instead of hiding DEBUG.

Done.